fix(KFLUXUI-301): remove workspace dependency from Secrets #113

StanislavJochman · 2025-02-11T15:13:07Z

Fixes

Description

Type of change

Screen shots / Gifs for design review

How to test or reproduce?

Browser conformance:

Chrome
Firefox
Safari
Edge

codecov · 2025-02-11T17:24:09Z

Codecov Report

Attention: Patch coverage is 88.13559% with 7 lines in your changes missing coverage. Please review.

Project coverage is 80.03%. Comparing base (591f649) to head (f01a64c).
Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
...View/tabs/TaskRunSecurityEnterpriseContractTab.tsx	0.00%	2 Missing ⚠️
...s/Commits/CommitDetails/tabs/CommitOverviewTab.tsx	0.00%	1 Missing ⚠️
src/components/GithubRedirect/GithubRedirect.tsx	0.00%	1 Missing ⚠️
.../PipelineRunDetailsView/PipelineRunDetailsView.tsx	0.00%	1 Missing ⚠️
...elineRunDetailsView/tabs/PipelineRunDetailsTab.tsx	0.00%	1 Missing ⚠️
...PipelineRunDetailsView/tabs/PipelineRunLogsTab.tsx	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #113      +/-   ##
==========================================
- Coverage   80.10%   80.03%   -0.08%     
==========================================
  Files         577      544      -33     
  Lines       21495    21132     -363     
  Branches     5064     5061       -3     
==========================================
- Hits        17219    16913     -306     
+ Misses       4252     4195      -57     
  Partials       24       24

Flag	Coverage Δ
unittests	`80.03% <88.13%> (-0.08%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...mponents/ApplicationDetails/ApplicationDetails.tsx	`83.33% <ø> (ø)`
.../visualization/hooks/useAppApplicationTestNodes.ts	`89.47% <ø> (-0.36%)`	⬇️
...s/overview/visualization/hooks/useAppBuildNodes.ts	`91.93% <100.00%> (-0.26%)`	⬇️
...rview/visualization/hooks/useAppComponentsNodes.ts	`89.58% <ø> (-0.42%)`	⬇️
src/components/Applications/ApplicationListRow.tsx	`92.30% <100.00%> (ø)`
...onents/Commits/CommitDetails/CommitDetailsView.tsx	`97.82% <ø> (ø)`
...mmits/CommitDetails/tabs/CommitsPipelineRunTab.tsx	`85.29% <100.00%> (ø)`
...mmitDetails/visualization/useCommitWorkflowData.ts	`91.95% <100.00%> (ø)`
src/components/Commits/commit-status.ts	`95.65% <100.00%> (ø)`
...nents/ComponentRelation/ComponentRelationModal.tsx	`91.48% <100.00%> (ø)`
... and 43 more

... and 34 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 591f649...f01a64c. Read the comment docs.

JoaoPedroPP

Please remove the comments that were left behind. Beside that I left three questions why we use workspace dependency in some parts of the code.

This PR will probably be affected by #111 and the path for Namespace dependency will to update.

src/components/Applications/ApplicationListRow.tsx

src/components/Commits/commit-status.ts

src/components/ComponentRelation/ComponentRelationModal.tsx

src/components/ComponentRelation/details-page/ComponentNudges.tsx

...omponents/ReleaseService/ReleasePlan/TriggerRelease/AddIssueSection/CVEComponentDropDown.tsx

src/components/Secrets/SecretsForm/ComponentDropdown.tsx

src/components/SnapshotDetails/__tests__/EnvironmentProvisionErrorAlert.spec.tsx

src/hooks/useComponents.ts

src/utils/tekton-results.ts

StanislavJochman · 2025-02-12T22:25:09Z

src/components/ComponentRelation/details-page/ComponentNudges.tsx

@@ -63,7 +64,7 @@ const ComponentNudgesSVG: React.FC<ComponentNudgesSVGprops> = ({
        {relatedComponents.map((comp, i) => (
          <li key={i} data-test="nudges-connector">
            <Link
-              to={`/workspaces/${workspace}/applications/${comp.spec?.application}/components/${comp.metadata.name}`}
+              to={`/workspaces/${namespace}/applications/${comp.spec?.application}/components/${comp.metadata.name}`}


Also should not be this like:

to={${APPLICATION_DETAILS_PATH.createPath({ workspaceName: namespace, applicationName: comp?.spec?.application, componentName: comp.metadata.name})}/} or something similar @JoaoPedroPP @CryptoRodeo

Nice catch!

Yeah, it should be the same

testcara · 2025-02-13T08:02:17Z

@StanislavJochman , could you help to attach screenshots or recordings? For me, it seems the patch fixed more than its required. The screenshots or recordings are helpful for general review.

JoaoPedroPP · 2025-02-13T12:34:35Z

src/components/ComponentRelation/details-page/__tests__/ComponentNudgesDependencies.spec.tsx

 jest.mock('../../../Workspace/useWorkspaceInfo', () => ({
  useWorkspaceInfo: jest.fn(() => ({ namespace: 'test-ns', workspace: 'test-ws' })),
 }));


Can we remove this?

JoaoPedroPP · 2025-02-13T12:56:32Z

src/utils/test-utils.tsx

+export const createUseNamespaceMock = (namespace: string = 'test-ns'): jest.Mock => {
+  const mockFn = jest.fn().mockReturnValue(namespace);
+
+  jest.spyOn(NameSpaceUtils, 'useNamespace').mockImplementation(mockFn);
+
+  beforeEach(() => {
+    mockFn.mockReturnValue(namespace);
+  });
+
+  return mockFn;
+};


There is a similar function in ./src/unit-test-utils/mock-namespace, please use mockUseNamespaceHook function.

JoaoPedroPP · 2025-02-14T12:56:50Z

src/components/CustomizedPipeline/CustomizeAllPipelines.tsx

@@ -60,7 +59,7 @@ const CustomizeAllPipelines: React.FC<React.PropsWithChildren<Props>> = ({
              component={(props) => (
                <Link
                  {...props}
-                  to={`/workspaces/${workspace}/import?application=${applicationName}`}
+                  to={`${IMPORT_PATH.createPath({ workspaceName: namespace })}?application=${applicationName}`}


Suggested change

to={`${IMPORT_PATH.createPath({ workspaceName: namespace })}?application=${applicationName}`}

to={APPLICATION_DETAILS_PATH.createPath({ workspaceName: namespace, applicationName })}

JoaoPedroPP · 2025-02-14T12:57:04Z

src/components/CustomizedPipeline/CustomizeAllPipelines.tsx

@@ -10,11 +10,11 @@ import {
 } from '@patternfly/react-core';
 import { useComponents } from '../../hooks/useComponents';
 import { ComponentModel } from '../../models';
+import { IMPORT_PATH } from '../../routes/paths';


Suggested change

import { IMPORT_PATH } from '../../routes/paths';

import { APPLICATION_DETAILS_PATH } from '../../routes/paths';

fix(KFLUXUI-301): remove workspace dependency from Secrets

d8fcf70

StanislavJochman marked this pull request as draft February 11, 2025 15:13

fix(KFLUXUI-301): removing all workspace dependencies

c462592

fix(KFLUXUI-301): removing unnecesarry deletions

cf6dabc

StanislavJochman requested review from sahil143 and CryptoRodeo and removed request for sahil143 February 12, 2025 16:02

StanislavJochman self-assigned this Feb 12, 2025

StanislavJochman requested review from sahil143 and testcara February 12, 2025 16:02

StanislavJochman marked this pull request as ready for review February 12, 2025 16:03

JoaoPedroPP requested changes Feb 12, 2025

View reviewed changes

StanislavJochman added 2 commits February 12, 2025 20:02

fix(KFLUXUI-301): removing comments

0d1d1ee

fix(KFLUXUI-301): fixing path

7261523

StanislavJochman requested a review from JoaoPedroPP February 12, 2025 21:52

CryptoRodeo reviewed Feb 12, 2025

View reviewed changes

src/utils/tekton-results.ts Outdated Show resolved Hide resolved

StanislavJochman commented Feb 12, 2025

View reviewed changes

fix(KFLUXUI-301): fixing reversed workspace and namespace

12055f4

StanislavJochman requested a review from CryptoRodeo February 12, 2025 22:58

fix(KFLUXUI-301): removing comment

f01a64c

StanislavJochman force-pushed the KFLUXUI-301 branch from 8cd65ce to f01a64c Compare February 12, 2025 23:27

JoaoPedroPP reviewed Feb 13, 2025

View reviewed changes

JoaoPedroPP reviewed Feb 14, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(KFLUXUI-301): remove workspace dependency from Secrets #113

fix(KFLUXUI-301): remove workspace dependency from Secrets #113

StanislavJochman commented Feb 11, 2025

codecov bot commented Feb 11, 2025 •

edited

Loading

JoaoPedroPP left a comment

StanislavJochman Feb 12, 2025

JoaoPedroPP Feb 13, 2025

testcara commented Feb 13, 2025

JoaoPedroPP Feb 13, 2025

JoaoPedroPP Feb 13, 2025 •

edited

Loading

JoaoPedroPP Feb 14, 2025

JoaoPedroPP Feb 14, 2025

	to={`${IMPORT_PATH.createPath({ workspaceName: namespace })}?application=${applicationName}`}
	to={APPLICATION_DETAILS_PATH.createPath({ workspaceName: namespace, applicationName })}

	import { IMPORT_PATH } from '../../routes/paths';
	import { APPLICATION_DETAILS_PATH } from '../../routes/paths';

fix(KFLUXUI-301): remove workspace dependency from Secrets #113

Are you sure you want to change the base?

fix(KFLUXUI-301): remove workspace dependency from Secrets #113

Conversation

StanislavJochman commented Feb 11, 2025

Fixes

Description

Type of change

Screen shots / Gifs for design review

How to test or reproduce?

Browser conformance:

codecov bot commented Feb 11, 2025 • edited Loading

Codecov Report

JoaoPedroPP left a comment

Choose a reason for hiding this comment

StanislavJochman Feb 12, 2025

Choose a reason for hiding this comment

JoaoPedroPP Feb 13, 2025

Choose a reason for hiding this comment

testcara commented Feb 13, 2025

JoaoPedroPP Feb 13, 2025

Choose a reason for hiding this comment

JoaoPedroPP Feb 13, 2025 • edited Loading

Choose a reason for hiding this comment

JoaoPedroPP Feb 14, 2025

Choose a reason for hiding this comment

JoaoPedroPP Feb 14, 2025

Choose a reason for hiding this comment

codecov bot commented Feb 11, 2025 •

edited

Loading

JoaoPedroPP Feb 13, 2025 •

edited

Loading