refactor(KFLUXUI-1002): rename Enterprise Contract to Conforma#708
refactor(KFLUXUI-1002): rename Enterprise Contract to Conforma#708marcin-michal wants to merge 4 commits intokonflux-ci:mainfrom
Conversation
marcin-michal
requested review from
rakshett and
sahil143
and removed request for
a team
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Conforma Table Components src/components/Conforma/ConformaTable/ConformaHeader.tsx, src/components/Conforma/ConformaTable/ConformaRow.tsx, src/components/Conforma/ConformaTable/ConformaExpandedRowContent.tsx, src/components/Conforma/ConformaTable/ConformaTable.tsx, src/components/Conforma/ConformaTable/ConformaTable.scss |
Introduces Conforma table header, row, expanded content, and main table. Prop/types updated to UIConformaData; CSS class names changed from ec-* to conforma-*; new column classes and sortable headers added. |
Conforma Types src/types/conforma.ts |
Renames and re-exports types/enums for Conforma (UIConformaData, ConformaResult, ComponentConformaResult, CONFORMA_RESULT_STATUS, etc.) replacing Enterprise Contract counterparts. |
Hooks & Result Mapping src/components/Conforma/useConformaResult.tsx, src/components/Conforma/utils.tsx, src/components/Conforma/__tests__/* |
Adds useConformaResult, useConformaResultFromLogs, and mapConformaResultData; renames parsing utilities to Conforma variants and updates internal names/return shapes to Conforma types. Tests updated accordingly. |
Conforma Utilities src/utils/conforma-utils.tsx, src/utils/__tests__/conforma-utils.spec.tsx |
Moves/updates rule/status helpers to use CONFORMA_RESULT_STATUS; getRuleStatus signature and display text updated; tests added for util behavior. |
Route & Tab Integrations src/components/PipelineRun/.../PipelineRunSecurityTab.tsx, src/components/TaskRunDetailsView/.../TaskRunSecurityTab.tsx, src/routes/page-routes/* |
Replaces Enterprise Contract tabs with Conforma equivalents in PipelineRun/TaskRun route and tab exports; route elements updated to render new Conforma tabs. |
Enterprise Contract Removals src/components/EnterpriseContract/..., src/models/enterprise-contract-policy.ts, src/types/enterprise-contract.-policy.ts |
Removes legacy EnterpriseContract table, header/row files, policy model exports, and Enterprise Contract-specific types and large mock data files. |
Tests Migration src/components/Conforma/__tests__/*, src/components/PipelineRun/.../__tests__/*, src/components/TaskRunDetailsView/.../__tests__/* |
Updates and adds tests to reflect Conforma names/hooks, replaces mocks and expectations previously tied to Enterprise Contract. |
Constants & Docs src/consts/documentation.ts, src/components/Conforma/__data__/mockConformaLogsJson.ts |
Replaces SECURITY_ENTERPRISE_CONTRACT_* constants with CONFORMA_* constants and updates mock data exports and statuses. |
Exports / Barrels src/models/index.ts, src/types/index.ts |
Removes enterprise-contract policy exports from barrels and switches type exports to conforma module where applicable. |
Minor UI/Permissions src/components/UserAccess/UserAccessForm/PermissionsTable.tsx |
Updates permission label text to include “Conforma (Enterprise Contract)” while keeping mapping structure unchanged. |
Sequence Diagram(s)
sequenceDiagram
participant Route as PipelineRun Route
participant Tab as SecurityConformaTab
participant Hook as useConformaResult
participant Table as ConformaTable
participant Row as WrappedConformaRow
Route->>Tab: render pipeline security tab
Tab->>Hook: useConformaResult(pipelineRunName)
Hook-->>Tab: [UIConformaData[], loaded, error]
Tab->>Table: <ConformaTable conformaResult={...}>
Table->>Row: render rows (WrappedConformaRow)
Row->>Table: expanded row -> ConformaExpandedRowContent
ConformaExpandedRowContent->>Table: external link -> CONFORMA_POLICY_AVAILABLE_RULE_COLLECTIONS_URL
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
- feat(KFLUXUI-1001): show Conforma results in pipeline security tab #661: Overlapping Conforma/Enterprise Contract refactor touching security/conforma feature surface and related symbols.
- chore(KFLUXUI-805): add units tests for pipelinerun logs and ec utils #481: Changes to utility functions (isResourceEnterpriseContract, getRuleStatus, parsing) that overlap with Conforma util renames.
- feat(/KFLUXUI-757): Migrated from taskRuns to taskRunsV2 in useEnterpriseContractResultFromLogs() hook #434: Modifies the result-from-logs hook (taskRunsV2) that may intersect with this PR's rename to useConformaResultFromLogs.
Suggested labels
ok-to-test
Suggested reviewers
- sahil143
- testcara
- JoaoPedroPP
- StanislavJochman
- rrosatti
A refactor sailed in with a Conforma name,
Types and tabs reorganized to play the game.
Rows and headers dressed anew,
Tests adjusted, links in view —
Conforma now leads where Enterprise once came.
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The pull request title accurately describes the primary change: a comprehensive rename of Enterprise Contract terminology to Conforma throughout the codebase. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
marcin-michal
requested review from
a team,
JoaoPedroPP,
StanislavJochman,
abhinandan13jan,
janaki29,
milantaky,
rrosatti and
testcara
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/components/TaskRunDetailsView/tabs/__tests__/TaskRunSecurityEnterpriseContractTab.spec.tsx (1)
1-115:⚠️ Potential issue | 🟡 MinorTest file name doesn't match the component under test.
Same issue as the PipelineRun test: this file is still named
TaskRunSecurityEnterpriseContractTab.spec.tsxbut now testsTaskRunSecurityTab. Rename toTaskRunSecurityTab.spec.tsxto follow the convention of naming test files after the component being tested. As per coding guidelines: "Name test files after the component being tested."src/utils/conforma-utils.tsx (1)
45-57:⚠️ Potential issue | 🟠 MajorFix error message on line 56 and address code duplication.
The error message on line 56 still references "EC results" instead of "Conforma results" for consistency with the renamed function.
Additionally, both
extractConformaResultsFromTaskRunLogsandgetRuleStatusare duplicated acrosssrc/utils/conforma-utils.tsxandsrc/components/Conforma/utils.tsx. Thesrc/components/Conforma/utils.tsxversions are actively used throughout the codebase (SecurityConformaTab, ConformaRow, useConformaResult), whilesrc/utils/conforma-utils.tsxversions are only referenced by their own test file. Consider consolidating to a single canonical location.Fix error message
- throw new Error(`Failed to parse EC results: ${error.message}`); + throw new Error(`Failed to parse Conforma results: ${error.message}`);src/components/Conforma/utils.tsx (2)
40-40:⚠️ Potential issue | 🟡 MinorTypo in comment: "formay" → "format".
Line 40:
This regex expect the logs from tekton results to be in this formayshould readformat.📝 Proposed fix
-/** - * This regex expect the logs from tekton results to be in this formay +/** + * This regex expects the logs from tekton results to be in this format
10-37:⚠️ Potential issue | 🟠 MajorConsolidate duplicate
getRuleStatusfunction;extractConformaResultsFromTaskRunLogsimplementations serve different log formats and should remain separate.The
getRuleStatusfunction is identical in both this file andsrc/utils/conforma-utils.tsx, creating unnecessary duplication. Move the implementation tosrc/utils/conforma-utils.tsx(where it's already tested) and import it here to follow DRY principles.However,
extractConformaResultsFromTaskRunLogslegitimately differs: this component version parsesstep-report-json :-format using a lookbehind regex, while the utils version parses[report-json]prefix format. Each has corresponding test coverage matching its log format. Keep both implementations separate.Also fix typo on line 40: "formay" → "format".
src/utils/__tests__/conforma-utils.spec.tsx (1)
194-194:⚠️ Potential issue | 🟡 MinorStale describe block name:
extractEcResultsFromTaskRunLogs.The function under test was renamed to
extractConformaResultsFromTaskRunLogs, but the describe block at line 194 still references the old name. Test descriptions at lines 215, 230, 258, and 288 also still say "EC results" — worth updating for grep-ability and consistency.📝 Proposed fix for the describe block
- describe('extractEcResultsFromTaskRunLogs', () => { + describe('extractConformaResultsFromTaskRunLogs', () => {
🤖 Fix all issues with AI agents
In `@src/components/Conforma/__tests__/utils.spec.ts`:
- Line 3: The test suite's describe label still reads
'extractEcResultsFromTaskRunLogs' but the function under test was renamed;
update the string in the describe(...) in
src/components/Conforma/__tests__/utils.spec.ts to match the current function
name used by the test cases (replace the stale 'extractEcResultsFromTaskRunLogs'
label with the new function name referenced in the file), so the describe block
correctly reflects the function being tested.
- Line 1: There are two implementations of
extractConformaResultsFromTaskRunLogs; remove the dead/unused one (the duplicate
implementation outside the Conforma component) and its associated test
(conforma-utils.spec.tsx), leaving only the active implementation used by
useConformaResult; ensure you delete the unused function declaration and its
test file, remove any exports/imports pointing to that removed module, and run
the test suite to confirm nothing else references
extractConformaResultsFromTaskRunLogs.
In
`@src/components/PipelineRun/PipelineRunDetailsView/tabs/__tests__/PipelineRunSecurityEnterpriseContractTab.spec.tsx`:
- Around line 1-63: The test file name doesn't match the component under
test—rename the file from PipelineRunSecurityEnterpriseContractTab.spec.tsx to
PipelineRunSecurityTab.spec.tsx and commit the rename; ensure any references
(e.g., in test-runner patterns, exports or related imports) are updated if
present, and run the test suite to confirm snapshots/coverage remain valid for
the PipelineRunSecurityTab tests.
🧹 Nitpick comments (9)
src/components/Conforma/useConformaResult.tsx (1)
141-141: Missed parameter rename:ecResultshould becrResultorconformaResult.The parameter name
ecResulton line 141 still uses the old "Enterprise Contract" abbreviation, inconsistent with the rest of the Conforma rename in this file (e.g.,crJson,crLoaded,conformaResult).Suggested fix
-export const mapConformaResultData = (ecResult: ComponentConformaResult[]): UIConformaData[] => { - return ecResult.reduce((acc, compResult) => { +export const mapConformaResultData = (crResult: ComponentConformaResult[]): UIConformaData[] => { + return crResult.reduce((acc, compResult) => {src/components/Conforma/__tests__/useConformaResult.spec.tsx (2)
62-62: Incorrectdescribeblock name — should beuseConformaResultFromLogs.This
describeblock (line 62) testsuseConformaResultFromLogs(see line 111), but is named'useConformaResult', which collides with the seconddescribeblock on line 328 that actually testsuseConformaResult. This makes test output ambiguous.Suggested fix
-describe('useConformaResult', () => { +describe('useConformaResultFromLogs', () => {
227-302: Leftoverec-prefixed variable names in test assertions.Several destructured variable names still use the old
ecprefix (e.g.,ecResulton lines 230/233,ec/ecLoadedon lines 241/244, 276/278, 299/301). These are inconsistent with the Conforma rename throughout the rest of this file.src/components/PipelineRun/PipelineRunDetailsView/PipelineRunDetailsView.tsx (1)
17-17: Inconsistent rename:isResourceEnterpriseContractstill uses old terminology.The function is now imported from
conforma-utilsbut retains theEnterpriseContractname. Consider renaming to something likeisResourceConformafor consistency with the broader rename effort in this PR, or add a comment explaining why it's intentionally kept.src/components/Conforma/ConformaTable/ConformaRow.tsx (1)
53-63:keyprop is ineffective here andfindIndexresult of -1 is unguarded.
ConformaRowis rendered as a single element (not inside a.map()), so thekeyprop has no reconciliation effect. More importantly, ifobjis not found insortedConformaResult,findIndexreturns-1and the component still renders — contradicting the apparent intent of the guard.Proposed fix
export const WrappedConformaRow: React.FC<WrappedConformaRowProps> = ({ obj, customData }) => { const customConformaResult = customData?.sortedConformaResult; - if (Array.isArray(customConformaResult) && customConformaResult.length > 0) { - const index = customConformaResult.findIndex((item) => item === obj); - - return <ConformaRow data={obj} key={index} />; + if (Array.isArray(customConformaResult) && customConformaResult.includes(obj)) { + return <ConformaRow data={obj} />; } return null; };src/utils/__tests__/conforma-utils.spec.tsx (1)
82-82:createECLoghelper not renamed to match Conforma convention.The helper at line 82 still uses
ECin its name. For consistency with the rest of the rename, considercreateConformaLog.📝 Proposed fix
- const createECLog = (data: ConformaResult): string => `[report-json] ${JSON.stringify(data)}`; + const createConformaLog = (data: ConformaResult): string => `[report-json] ${JSON.stringify(data)}`;Then update all call sites within this file.
src/components/Conforma/ConformaTable/ConformaTable.tsx (2)
67-71:crResult.sort(...)mutates the prop array in place.
Array.prototype.sortis in-place. Sorting a prop directly insideuseMemomutates the parent's array, which can cause subtle React bugs (stale references, skipped re-renders). Consider spreading first:📝 Proposed fix
const sortedCRResult = React.useMemo(() => { return crResult - ? crResult.sort(getSortColumnFuntion(COLUMN_ORDER[activeSortIndex], activeSortDirection)) + ? [...crResult].sort(getSortColumnFuntion(COLUMN_ORDER[activeSortIndex], activeSortDirection)) : undefined; }, [activeSortDirection, activeSortIndex, crResult]);
91-96: InconsistentcustomDatakey naming between Table and Row components.The Table-level
customDataprop passes{{ sortedCRResult }}, while the Row override passes{{ sortedConformaResult: sortedCRResult }}. SinceWrappedConformaRowexpects thesortedConformaResultkey and the Row explicitly overrides the customData, the Table-level prop with keysortedCRResultis unused. Align the keys for consistency:📝 Proposed fix
loaded - customData={{ sortedCRResult }} + customData={{ sortedConformaResult: sortedCRResult }} expandsrc/components/Conforma/SecurityConformaTab.tsx (1)
32-44:getResultsSummarylacks parameter types.The function parameters
CRsandcrLoadedare untyped. Adding types would align with the strict TypeScript guidelines.📝 Proposed fix
-const getResultsSummary = (CRs, crLoaded) => { +const getResultsSummary = (CRs: UIConformaData[] | undefined, crLoaded: boolean) => {
...components/PipelineRun/PipelineRunDetailsView/tabs/__tests__/PipelineRunSecurityTab.spec.tsx
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #708 +/- ##
==========================================
- Coverage 87.23% 87.12% -0.11%
==========================================
Files 764 761 -3
Lines 58225 58088 -137
Branches 5658 6875 +1217
==========================================
- Hits 50792 50610 -182
+ Misses 7376 7356 -20
- Partials 57 122 +65
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 64 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Assisted-by: Cursor
marcin-michal
force-pushed
the
KFLUXUI-1002/conforma-rename
branch
from
2527cbb to
d5ffeb9
Compare
rrosatti
left a comment
rrosatti
left a comment
There was a problem hiding this comment.
LGTM! I'd just suggest to delete the src/components/Conforma/__data__/mockConformaPolicies.ts file, it seems to not being used anywhere :)
Assisted-by: Cursor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/Conforma/utils.tsx (1)
40-40:⚠️ Potential issue | 🟡 MinorTypo in JSDoc comment: "formay" → "format".
✏️ Fix
- * This regex expect the logs from tekton results to be in this formay + * This regex expects the logs from tekton results to be in this format🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Conforma/utils.tsx` at line 40, Fix the typo in the JSDoc above the regex that parses Tekton results: change "formay" to "format" in the comment (the JSDoc immediately preceding the regex in Conforma's utils.tsx).
🧹 Nitpick comments (9)
src/components/Conforma/useConformaResult.tsx (1)
87-88:eslint-disable-next-line no-consolesuppresses a lint rule rather than fixing the root cause.Both
console.warncalls disableno-consoleinline. Per project conventions, consider using a structured logging abstraction or moving these to an appropriate observability hook instead of silencing the rule.Also applies to: 110-111
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Conforma/useConformaResult.tsx` around lines 87 - 88, Replace the inline console.warn calls (and the accompanying eslint-disable comments) in useConformaResult (file: src/components/Conforma/useConformaResult.tsx, occurrences around the current console.warn at ~87 and ~110) with the project's structured logging/observability mechanism: import and call the existing logger or observability hook (e.g. processLogger or useLogger) to emit a warning/error with the error object and contextual fields, or accept a logger via hook params and use that; remove the eslint-disable comments after switching to the logger so no lint rules are suppressed.src/components/Conforma/ConformaTable/__tests__/ConformaExpandedRowContent.spec.tsx (1)
29-42: LGTM — minor optional coverage gap forsolutionfield.The two test cases correctly cover the render/no-render paths. The
invalidContentfixture exercises the early-return guard correctly (null?.lengthis falsy). Thesolutionfield rendered byConformaExpandedRowContenthas no dedicated test case, but this is a low-risk gap since the field is optional.✏️ Optional: add a solution field test
it('should render the component', () => { render(<ConformaExpandedRowContent obj={rowContent} />); screen.getByText('Effective from'); screen.getByText('Collection'); screen.getByText('abcd, efg'); screen.getByText('Rule Description'); screen.getByText('dummy description'); }); + it('should render solution when present', () => { + render(<ConformaExpandedRowContent obj={{ ...rowContent, solution: 'Fix the issue' }} />); + screen.getByText('Solution'); + screen.getByText('Fix the issue'); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Conforma/ConformaTable/__tests__/ConformaExpandedRowContent.spec.tsx` around lines 29 - 42, Add a small test in ConformaExpandedRowContent.spec.tsx to assert the optional "solution" field renders when present: render the component with a fixture that includes a non-empty solution and use screen.getByText to verify the solution string appears (and conversely ensure it is absent for invalidContent if desired). Target the ConformaExpandedRowContent component and the existing test harness (render/screen) so the new assertion mirrors the style of the existing "Rule Description" / "dummy description" checks.src/utils/__tests__/conforma-utils.spec.tsx (1)
97-138:container.querySelector('svg')is a fragile, implementation-detail query.Per project test guidelines, semantic queries (
getByRole,getByLabelText) are preferred over structural DOM queries. PatternFly icons can exposerole="img"or an accessible title; querying for raw SVG elements is brittle if the icon library changes its rendering.✏️ Example using accessible text assertion instead
- expect(result.container.querySelector('svg')).toBeInTheDocument(); - expect(result.container).toHaveTextContent('Success'); + expect(result.getByText('Success')).toBeInTheDocument();If the icons must be asserted, use
getByRole('img', { hidden: true })to query by accessible role rather than by tag name.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/__tests__/conforma-utils.spec.tsx` around lines 97 - 138, The tests assert icon presence by querying raw SVGs which is brittle; update the getRuleStatus tests to use semantic queries instead (e.g., replace expect(result.container.querySelector('svg')).toBeInTheDocument() with expect(result.getByRole('img', { hidden: true })).toBeInTheDocument() or getByLabelText/getByTitle if the icon exposes accessible text), keeping the rest of the assertions (text content like 'Success', 'Failed', 'Warning', 'Missing') the same; apply this change to each test that checks the icon for getRuleStatus and for any null/undefined edge-case tests that assert icon presence.src/components/Conforma/utils.tsx (1)
10-37:getRuleStatusis duplicated withsrc/utils/conforma-utils.tsx.Both files export an identical implementation of
getRuleStatus. The component-levelutils.tsxshould import from the shared utility instead of maintaining its own copy.♻️ Proposed fix: remove the duplicate, import from the shared util
-import { CheckCircleIcon } from '@patternfly/react-icons/dist/esm/icons/check-circle-icon'; -import { DotCircleIcon } from '@patternfly/react-icons/dist/esm/icons/dot-circle-icon'; -import { ExclamationCircleIcon } from '@patternfly/react-icons/dist/esm/icons/exclamation-circle-icon'; -import { ExclamationTriangleIcon } from '@patternfly/react-icons/dist/esm/icons/exclamation-triangle-icon'; -import { global_danger_color_100 as redColor } from '@patternfly/react-tokens/dist/js/global_danger_color_100'; -import { global_success_color_100 as greenColor } from '@patternfly/react-tokens/dist/js/global_success_color_100'; -import { global_warning_color_100 as yellowColor } from '@patternfly/react-tokens/dist/js/global_warning_color_100'; import { CONFORMA_RESULT_STATUS, ConformaResult } from '~/types/conforma'; +export { getRuleStatus } from '~/utils/conforma-utils'; -export const getRuleStatus = (type: CONFORMA_RESULT_STATUS) => { - switch (type) { - ... - } -};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Conforma/utils.tsx` around lines 10 - 37, Remove the duplicated getRuleStatus implementation in the component-level utils and instead import the shared getRuleStatus from the existing shared utility; delete the local function (and any identical icon/color constants it only used) and add an import for the shared getRuleStatus so the component uses that exported function (ensure the shared module exports getRuleStatus and that the referenced symbols CONFORMA_RESULT_STATUS, CheckCircleIcon, ExclamationCircleIcon, ExclamationTriangleIcon, DotCircleIcon, greenColor, redColor, yellowColor are provided by the shared implementation or imported where needed).src/components/Conforma/__tests__/useConformaResult.spec.tsx (1)
62-62: Misleadingdescribeblock name — testsuseConformaResultFromLogs, notuseConformaResult.This describe block (line 62) tests
useConformaResultFromLogs(see therenderHookWithQueryClientat line 111), but it's named'useConformaResult'— the same name as the second describe block at line 328 which tests the actualuseConformaResulthook. This creates ambiguous test output and makes failures harder to triage.Proposed fix
-describe('useConformaResult', () => { +describe('useConformaResultFromLogs', () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Conforma/__tests__/useConformaResult.spec.tsx` at line 62, The describe block currently named 'useConformaResult' actually contains tests for useConformaResultFromLogs; rename that top-level describe to 'useConformaResultFromLogs' (and update any nested test titles if they reference the old name) so test output is unambiguous; locate the block that calls renderHookWithQueryClient and references useConformaResultFromLogs and change the describe string to match the hook under test, keeping the existing test bodies intact.src/components/Conforma/SecurityConformaTab.tsx (2)
32-43: Missing TypeScript types ongetResultsSummaryparameters.
CRsandcrLoadedare untyped. Since this is in changed lines, consider adding types for consistency with the strict TypeScript guideline.Proposed fix
-const getResultsSummary = (CRs, crLoaded) => { +const getResultsSummary = (CRs: UIConformaData[] | undefined, crLoaded: boolean) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Conforma/SecurityConformaTab.tsx` around lines 32 - 43, getResultsSummary currently accepts untyped parameters CRs and crLoaded; add explicit TypeScript types: type CRs as an array of objects that include a status: string property (e.g., Array<{ status: string }>) or a more specific interface if one exists, type crLoaded as boolean, and annotate the function return as Record<string, number> (or the appropriate mapped type). Update the getResultsSummary signature to use these types so TypeScript can validate uses of cr.status and the returned statusFilter; keep using the existing statuses variable and reduce logic unchanged.
24-24: Remove duplicategetRuleStatusfunction and use shared implementation.The
getRuleStatusfunction is defined identically in bothsrc/components/Conforma/utils.tsxandsrc/utils/conforma-utils.tsx. The canonical version exists insrc/utils/conforma-utils.tsx, and bothSecurityConformaTab.tsxandConformaRow.tsxshould import from there instead of maintaining a duplicate in the local utils file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Conforma/SecurityConformaTab.tsx` at line 24, Remove the duplicate getRuleStatus implementation in the local component utils (src/components/Conforma/utils.tsx) and update imports in SecurityConformaTab.tsx and ConformaRow.tsx to import getRuleStatus from the shared canonical module (src/utils/conforma-utils.tsx); ensure both files replace any local import like "import { getRuleStatus } from './utils';" with the shared module import and delete the duplicate function definition from the component utils file so only the single shared getRuleStatus symbol is used across components.src/components/PipelineRun/PipelineRunDetailsView/tabs/PipelineRunSecurityTab.tsx (1)
5-5: Use the~/componentspath alias for consistency.Line 3 already uses the
~alias for an internal import; line 5 (new) goes back to a relative path.♻️ Proposed fix
-import { SecurityConformaTab } from '../../../Conforma/SecurityConformaTab'; +import { SecurityConformaTab } from '~/components/Conforma/SecurityConformaTab';As per coding guidelines: "Use absolute imports with configured path aliases:
~/components,~/types,~/k8s,~/utils,~/models,@routes."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/PipelineRun/PipelineRunDetailsView/tabs/PipelineRunSecurityTab.tsx` at line 5, Replace the relative import of SecurityConformaTab with the project path-alias import to match existing style: update the import for SecurityConformaTab (symbol SecurityConformaTab in PipelineRunSecurityTab.tsx) to use the ~/components alias instead of '../../../Conforma/SecurityConformaTab' so it follows the established absolute import patterns (e.g., ~/components).src/components/TaskRunDetailsView/tabs/__tests__/TaskRunSecurityTab.spec.tsx (1)
39-57: Misleading test name — scenario is loading state, not error.
mockUseConformaResult.mockReturnValue([null, false, undefined])givescrLoaded=false, crError=undefined, which produces the spinner (loading) path inSecurityConformaTab, not an error path. An error scenario would require a truthy third value (e.g.,new Error('...')).♻️ Proposed rename
- it('should handle error when conforma fails to load', () => { + it('should show nothing when conforma data has not loaded yet', () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TaskRunDetailsView/tabs/__tests__/TaskRunSecurityTab.spec.tsx` around lines 39 - 57, The test name is misleading: mockUseConformaResult currently returns [null, false, undefined] which represents a loading state (crLoaded=false, crError=undefined) and triggers the spinner in SecurityConformaTab; either rename the test "should handle loading when Conforma is not yet loaded" to reflect that, or change the mock to simulate an error (return a truthy error as the third tuple value, e.g., an Error instance) so the test exercises the error branch in SecurityConformaTab; update assertions accordingly (if simulating error, assert the error UI is shown and the loading spinner is not).
🤖 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/Conforma/__tests__/useConformaResult.spec.tsx`:
- Around line 345-351: The tests in the second describe('useConformaResult')
block lose the namespace spy because jest.clearAllMocks() resets it; add a call
to mockUseNamespaceHook('test-ns') inside that block's beforeEach (alongside
queryClient = createTestQueryClient(), jest.clearAllMocks(), etc.) so
useConformaResult sees the mocked namespace during those tests and the spy is
re-established.
In `@src/components/Conforma/ConformaTable/ConformaTable.tsx`:
- Around line 67-71: The current sortedCRResult useMemo calls
crResult.sort(...), which mutates the crResult prop; fix by copying before
sorting (e.g. replace crResult.sort(...) with [...crResult].sort(...) or
crResult.slice().sort(...)) so sortedCRResult is derived from an immutable copy;
keep the same getSortColumnFuntion(COLUMN_ORDER[activeSortIndex],
activeSortDirection) call and dependency array.
In `@src/components/Conforma/useConformaResult.tsx`:
- Around line 141-142: The parameter name ecResult in mapConformaResultData is
stale; rename it to a clearer name like conformaResults (or conformaResultList)
and update all uses within the function (e.g., the reduce call and any
references to compResult) so the parameter and its internal references reflect
the new Conforma naming and remove the old "ec" prefix.
In
`@src/components/PipelineRun/PipelineRunDetailsView/tabs/PipelineRunSecurityTab.tsx`:
- Line 12: pipelineRunName coming from useParams<RouterParams>() is typed as
string | undefined but SecurityConformaTab and useConformaResult expect a
string; update the call site in PipelineRunSecurityTab.tsx to either
guard/assert before passing it: obtain const { pipelineRunName } =
useParams<RouterParams>(); then either throw or return early if pipelineRunName
is undefined, or pass pipelineRunName! (non-null assertion) to
SecurityConformaTab, ensuring SecurityConformaTab and its
useConformaResult(pipelineRunName: string) receive a guaranteed string; choose a
guard (e.g., render fallback/error when undefined) or a single non-null
assertion to satisfy the type checker.
In `@src/types/conforma.ts`:
- Line 30: Remove the dead type alias ConformaResultType: find and delete the
export type ConformaResultType = 'violations' | 'successes' | 'warnings';
declaration in src/types/conforma.ts so the codebase relies solely on the
existing CONFORMA_RESULT_STATUS enum; ensure no other files reference
ConformaResultType (run a quick project-wide search) and keep
CONFORMA_RESULT_STATUS as the single source of truth.
---
Outside diff comments:
In `@src/components/Conforma/utils.tsx`:
- Line 40: Fix the typo in the JSDoc above the regex that parses Tekton results:
change "formay" to "format" in the comment (the JSDoc immediately preceding the
regex in Conforma's utils.tsx).
---
Nitpick comments:
In `@src/components/Conforma/__tests__/useConformaResult.spec.tsx`:
- Line 62: The describe block currently named 'useConformaResult' actually
contains tests for useConformaResultFromLogs; rename that top-level describe to
'useConformaResultFromLogs' (and update any nested test titles if they reference
the old name) so test output is unambiguous; locate the block that calls
renderHookWithQueryClient and references useConformaResultFromLogs and change
the describe string to match the hook under test, keeping the existing test
bodies intact.
In
`@src/components/Conforma/ConformaTable/__tests__/ConformaExpandedRowContent.spec.tsx`:
- Around line 29-42: Add a small test in ConformaExpandedRowContent.spec.tsx to
assert the optional "solution" field renders when present: render the component
with a fixture that includes a non-empty solution and use screen.getByText to
verify the solution string appears (and conversely ensure it is absent for
invalidContent if desired). Target the ConformaExpandedRowContent component and
the existing test harness (render/screen) so the new assertion mirrors the style
of the existing "Rule Description" / "dummy description" checks.
In `@src/components/Conforma/SecurityConformaTab.tsx`:
- Around line 32-43: getResultsSummary currently accepts untyped parameters CRs
and crLoaded; add explicit TypeScript types: type CRs as an array of objects
that include a status: string property (e.g., Array<{ status: string }>) or a
more specific interface if one exists, type crLoaded as boolean, and annotate
the function return as Record<string, number> (or the appropriate mapped type).
Update the getResultsSummary signature to use these types so TypeScript can
validate uses of cr.status and the returned statusFilter; keep using the
existing statuses variable and reduce logic unchanged.
- Line 24: Remove the duplicate getRuleStatus implementation in the local
component utils (src/components/Conforma/utils.tsx) and update imports in
SecurityConformaTab.tsx and ConformaRow.tsx to import getRuleStatus from the
shared canonical module (src/utils/conforma-utils.tsx); ensure both files
replace any local import like "import { getRuleStatus } from './utils';" with
the shared module import and delete the duplicate function definition from the
component utils file so only the single shared getRuleStatus symbol is used
across components.
In `@src/components/Conforma/useConformaResult.tsx`:
- Around line 87-88: Replace the inline console.warn calls (and the accompanying
eslint-disable comments) in useConformaResult (file:
src/components/Conforma/useConformaResult.tsx, occurrences around the current
console.warn at ~87 and ~110) with the project's structured
logging/observability mechanism: import and call the existing logger or
observability hook (e.g. processLogger or useLogger) to emit a warning/error
with the error object and contextual fields, or accept a logger via hook params
and use that; remove the eslint-disable comments after switching to the logger
so no lint rules are suppressed.
In `@src/components/Conforma/utils.tsx`:
- Around line 10-37: Remove the duplicated getRuleStatus implementation in the
component-level utils and instead import the shared getRuleStatus from the
existing shared utility; delete the local function (and any identical icon/color
constants it only used) and add an import for the shared getRuleStatus so the
component uses that exported function (ensure the shared module exports
getRuleStatus and that the referenced symbols CONFORMA_RESULT_STATUS,
CheckCircleIcon, ExclamationCircleIcon, ExclamationTriangleIcon, DotCircleIcon,
greenColor, redColor, yellowColor are provided by the shared implementation or
imported where needed).
In
`@src/components/PipelineRun/PipelineRunDetailsView/tabs/PipelineRunSecurityTab.tsx`:
- Line 5: Replace the relative import of SecurityConformaTab with the project
path-alias import to match existing style: update the import for
SecurityConformaTab (symbol SecurityConformaTab in PipelineRunSecurityTab.tsx)
to use the ~/components alias instead of '../../../Conforma/SecurityConformaTab'
so it follows the established absolute import patterns (e.g., ~/components).
In
`@src/components/TaskRunDetailsView/tabs/__tests__/TaskRunSecurityTab.spec.tsx`:
- Around line 39-57: The test name is misleading: mockUseConformaResult
currently returns [null, false, undefined] which represents a loading state
(crLoaded=false, crError=undefined) and triggers the spinner in
SecurityConformaTab; either rename the test "should handle loading when Conforma
is not yet loaded" to reflect that, or change the mock to simulate an error
(return a truthy error as the third tuple value, e.g., an Error instance) so the
test exercises the error branch in SecurityConformaTab; update assertions
accordingly (if simulating error, assert the error UI is shown and the loading
spinner is not).
In `@src/utils/__tests__/conforma-utils.spec.tsx`:
- Around line 97-138: The tests assert icon presence by querying raw SVGs which
is brittle; update the getRuleStatus tests to use semantic queries instead
(e.g., replace expect(result.container.querySelector('svg')).toBeInTheDocument()
with expect(result.getByRole('img', { hidden: true })).toBeInTheDocument() or
getByLabelText/getByTitle if the icon exposes accessible text), keeping the rest
of the assertions (text content like 'Success', 'Failed', 'Warning', 'Missing')
the same; apply this change to each test that checks the icon for getRuleStatus
and for any null/undefined edge-case tests that assert icon presence.
src/components/PipelineRun/PipelineRunDetailsView/tabs/PipelineRunSecurityTab.tsx
Show resolved
Hide resolved
Assisted-by: Cursor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/components/Conforma/__tests__/useConformaResult.spec.tsx (3)
65-77:mockEnterpriseContractPipelineRunis duplicated across bothdescribeblocks.The fixture is defined identically in both blocks. Extract it to file scope to avoid the duplication.
✏️ Proposed fix
Move the shared fixture before the first
describeblock (file scope) and remove the duplicate in the second block:+const mockEnterpriseContractPipelineRun = { + metadata: { + name: 'test-pipelinerun', + labels: { [ENTERPRISE_CONTRACT_LABEL]: 'enterprise-contract' }, + }, + status: { pipelineSpec: { tasks: [{ name: EC_TASK }] } }, +}; + describe('useConformaResultFromLogs', () => { - const mockEnterpriseContractPipelineRun = { ... }; ... }); describe('useConformaResult', () => { - const mockEnterpriseContractPipelineRun = { ... }; ... });Also applies to: 331-343
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Conforma/__tests__/useConformaResult.spec.tsx` around lines 65 - 77, The test fixture mockEnterpriseContractPipelineRun (which references ENTERPRISE_CONTRACT_LABEL and EC_TASK) is duplicated in both describe blocks; move its declaration out to file scope above the first describe so both tests reuse the same object and delete the duplicate inside the second describe, ensuring imports/constants (ENTERPRISE_CONTRACT_LABEL, EC_TASK) are still in scope and updating any references if renamed.
62-62: Firstdescribeblock is mislabeled — should beuseConformaResultFromLogs.The block at line 62 is named
'useConformaResult'but every test inside it rendersuseConformaResultFromLogs(line 111). This causes ambiguity since there is also a seconddescribe('useConformaResult')block at line 328 that correctly tests theuseConformaResulthook. Test output will show two suites with the same name.✏️ Proposed fix
-describe('useConformaResult', () => { +describe('useConformaResultFromLogs', () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Conforma/__tests__/useConformaResult.spec.tsx` at line 62, The top-level describe string is wrong: rename the first describe that currently reads 'useConformaResult' to 'useConformaResultFromLogs' so it matches the hook under test (useConformaResultFromLogs) and avoids colliding with the other describe for useConformaResult; locate the describe(...) call in the test file surrounding the tests that render useConformaResultFromLogs and update its label accordingly.
227-244: StaleecResult/ec/ecLoadeddestructuring names.Several test destructurings still use the old EC-prefixed names. These don't affect test correctness but contradict the rename and mislead readers.
✏️ Proposed fix
- const [ecResult, loaded] = result.current; // line 230 + const [conformaResult, loaded] = result.current; - const [ecResult, loaded] = result.current; // line 241 + const [conformaResult, loaded] = result.current; - const [ecResult, loaded] = result.current; // line 276 + const [conformaResult, loaded] = result.current; - const [ec, ecLoaded] = result.current; // line 299 + const [conformaResult, crLoaded] = result.current;Also applies to: 276-278, 299-302
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Conforma/__tests__/useConformaResult.spec.tsx` around lines 227 - 244, Tests still destructure using legacy EC-prefixed names (ecResult / ec / ecLoaded) which should be updated to the new Conforma names; update all occurrences where the hook result is destructured (e.g., in the it blocks using renderHookWithQueryClient and mockCommmonFetchJSON) to use the current naming convention (for example replace ecResult/ec/ecLoaded with conformaResult/loaded or result/loaded as used elsewhere), and apply the same rename to the other instances mentioned (around the other test blocks referenced) so variable names are consistent and no stale EC-prefixed identifiers remain.src/components/Conforma/ConformaTable/ConformaTable.tsx (2)
21-21: Fix typo in exported function name:getSortColumnFuntion→getSortColumnFunction.The misspelled function is defined at line 21 and called once at line 69 within the same file, so the rename scope is limited to ConformaTable.tsx.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Conforma/ConformaTable/ConformaTable.tsx` at line 21, Rename the misspelled exported function getSortColumnFuntion to getSortColumnFunction and update every internal reference to it (including the call where getSortColumnFuntion is invoked) so the export and usages match; ensure the exported identifier and any imports/usages within ConformaTable.tsx are updated to the corrected name.
84-96: Line 96'scustomDataprop is misleading and functionally unused.The Table component receives
customData={{ sortedCRResult }}with keysortedCRResult, but the Row callback (lines 84–93) provides its owncustomData={{ sortedConformaResult: sortedCRResult }}with the correct key that WrappedConformaRow expects (sortedConformaResult). This override prevents a runtime bug, but it also means line 96'scustomDatais never consumed.Either remove line 96's
customDataprop (since Table doesn't require it) or align the key name to avoid confusion:✏️ Proposed fix
- customData={{ sortedCRResult }} + customData={{ sortedConformaResult: sortedCRResult }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Conforma/ConformaTable/ConformaTable.tsx` around lines 84 - 96, The Table is being passed a top-level prop customData={{ sortedCRResult }} that is never used because the Row renderer supplies its own customData with the expected key sortedConformaResult; remove the redundant top-level customData prop (the one using sortedCRResult) from the Table props in ConformaTable.tsx, or if you prefer keeping it, rename its key to sortedConformaResult so it matches what WrappedConformaRow expects (reference the Row callback and WrappedConformaRow, and the variables sortedCRResult and sortedConformaResult).
🤖 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/Conforma/useConformaResult.tsx`:
- Around line 87-88: In useConformaResult (the catch blocks inside the async
fetch flow), remove the eslint-disable comment and replace console.warn('Error
while fetching Conforma result from logs', err) (and the similar console.warn at
lines ~110-111) with the project's logging utility (e.g., call logger.warn or
processLogger.warn consistent with the repo's logger API) so the error and
context are recorded via the central logger; if the function already returns the
error upward, alternatively remove the log entirely and propagate the
error—ensure the import/use of the project's logger is added and used in those
catch blocks instead of console.warn.
---
Duplicate comments:
In `@src/components/Conforma/__tests__/useConformaResult.spec.tsx`:
- Around line 345-352: There's a duplicate call to
mockUseNamespaceHook('test-ns') in the second describe's beforeEach; remove the
redundant invocation (or hoist a single call to the outer beforeEach) so
mockUseNamespaceHook is only set once; update the beforeEach that currently
calls mockUseNamespaceHook in the second describe and keep only the single,
authoritative call to mockUseNamespaceHook('test-ns') to avoid duplicate
mocking.
In `@src/components/Conforma/ConformaTable/ConformaTable.tsx`:
- Around line 67-71: The reviewer noted the in-place sort mutation is already
addressed; remove the duplicate review comment and ensure the memoized
sortedCRResult still uses a non-mutating copy (the [...crResult].sort(...)
pattern) and that the comparator function reference (getSortColumnFuntion) is
correct where used; no behavioral change to sortedCRResult is needed—just clean
up the duplicate comment and verify the comparator name is the intended symbol.
In `@src/components/Conforma/useConformaResult.tsx`:
- Around line 141-142: The function signature for mapConformaResultData still
references the old parameter name in comments or internal references; update any
remaining uses or inline comments that mention ecResult to use the new parameter
name conformaResult, and ensure all references inside the mapConformaResultData
function body (and any tests/helpers calling it) read from the conformaResult
parameter to avoid stale identifier mismatches.
---
Nitpick comments:
In `@src/components/Conforma/__tests__/useConformaResult.spec.tsx`:
- Around line 65-77: The test fixture mockEnterpriseContractPipelineRun (which
references ENTERPRISE_CONTRACT_LABEL and EC_TASK) is duplicated in both describe
blocks; move its declaration out to file scope above the first describe so both
tests reuse the same object and delete the duplicate inside the second describe,
ensuring imports/constants (ENTERPRISE_CONTRACT_LABEL, EC_TASK) are still in
scope and updating any references if renamed.
- Line 62: The top-level describe string is wrong: rename the first describe
that currently reads 'useConformaResult' to 'useConformaResultFromLogs' so it
matches the hook under test (useConformaResultFromLogs) and avoids colliding
with the other describe for useConformaResult; locate the describe(...) call in
the test file surrounding the tests that render useConformaResultFromLogs and
update its label accordingly.
- Around line 227-244: Tests still destructure using legacy EC-prefixed names
(ecResult / ec / ecLoaded) which should be updated to the new Conforma names;
update all occurrences where the hook result is destructured (e.g., in the it
blocks using renderHookWithQueryClient and mockCommmonFetchJSON) to use the
current naming convention (for example replace ecResult/ec/ecLoaded with
conformaResult/loaded or result/loaded as used elsewhere), and apply the same
rename to the other instances mentioned (around the other test blocks
referenced) so variable names are consistent and no stale EC-prefixed
identifiers remain.
In `@src/components/Conforma/ConformaTable/ConformaTable.tsx`:
- Line 21: Rename the misspelled exported function getSortColumnFuntion to
getSortColumnFunction and update every internal reference to it (including the
call where getSortColumnFuntion is invoked) so the export and usages match;
ensure the exported identifier and any imports/usages within ConformaTable.tsx
are updated to the corrected name.
- Around line 84-96: The Table is being passed a top-level prop customData={{
sortedCRResult }} that is never used because the Row renderer supplies its own
customData with the expected key sortedConformaResult; remove the redundant
top-level customData prop (the one using sortedCRResult) from the Table props in
ConformaTable.tsx, or if you prefer keeping it, rename its key to
sortedConformaResult so it matches what WrappedConformaRow expects (reference
the Row callback and WrappedConformaRow, and the variables sortedCRResult and
sortedConformaResult).
3 participants
Assisted-by: Cursor
Fixes
KFLUXUI-1002
Description
Renaming mentions of
Enterprise ContracttoConformaas it has been renamed https://conforma.dev/posts/whats-in-a-name/Some mentions of Enterprise Contract are left as is, as some pipeline runs still use this old name (for example https://konflux-ui.apps.stone-prd-rh01.pg1f.p1.openshiftapps.com/ns/konflux-ui-tenant/applications/konflux-ui/pipelineruns/ec-k2wsc).
Some dead code related to ECs was removed as well.
Type of change
Summary by CodeRabbit
New Features
Bug Fixes
Refactor