CONSOLE-4697: Enable strickNullCheck for /console-app/components/resource-quota | quick-starts | tour | tabs | vpa | storage #15418

krishagarwal278 · 2025-08-19T16:03:24Z

Fixed SNC errors in storage, resource-quota, quick-starts, tour, tab, vpa

openshift-ci-robot · 2025-08-19T16:03:28Z

@krishagarwal278: This pull request references CONSOLE-4697 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the sub-task to target the "4.20.0" version, but no target version was set.

In response to this:

Fixed SNC errors in storage, resource-quota, quick-starts, tour, tab, vpa

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

krishagarwal278 · 2025-08-19T16:05:13Z

/label docs-approved
/label px-approved

krishagarwal278 · 2025-08-20T18:26:29Z

/retest

cajieh

Nice work so far!: 👍
I did a first pass, and added comments. Let me know if you have any questions.

cajieh · 2025-08-20T16:26:47Z

frontend/packages/console-app/src/components/quick-starts/QuickStartConfiguration.tsx

@@ -169,7 +169,7 @@ const QuickStartConfiguration: React.FC<{ readonly: boolean }> = ({ readonly })
      />

      <LoadError error={consoleConfigError} />
-      <SaveStatus {...saveStatus} />
+      <SaveStatus {...(saveStatus as SaveStatusProps)} />


I think we can avoid the use of as here by first checking for saveStatus. The type assertions with as should be last resort solutions, not quick fixes for type errors.

{saveStatus && <SaveStatus {...saveStatus} />}

cajieh · 2025-08-20T16:42:12Z

frontend/packages/console-app/src/components/quick-starts/utils/quick-start-context.tsx

@@ -40,7 +40,7 @@ const QUICKSTART_REDUX_STATE_LOCAL_STORAGE_KEY = 'bridge/quick-start-redux-state

 const getInitialState = () =>
  localStorage.getItem(QUICKSTART_REDUX_STATE_LOCAL_STORAGE_KEY)
-    ? JSON.parse(localStorage.getItem(QUICKSTART_REDUX_STATE_LOCAL_STORAGE_KEY))
+    ? JSON.parse(localStorage.getItem(QUICKSTART_REDUX_STATE_LOCAL_STORAGE_KEY) || '{}')


How about we refactor this to be a bit cleaner?

const getInitialState = () => { const stored = localStorage.getItem(QUICKSTART_REDUX_STATE_LOCAL_STORAGE_KEY); return stored ? JSON.parse(stored) : {}; };

cajieh · 2025-08-20T17:22:36Z

frontend/packages/console-app/src/components/quick-starts/utils/useQuickStarts.ts

@@ -117,5 +117,5 @@ export const useQuickStarts = (filterDisabledQuickStarts = true): WatchK8sResult
    );
  }, [quickStarts, quickStartsLoaded, filterDisabledQuickStarts, preferredLanguage]);

-  return [bestMatchQuickStarts, quickStartsLoaded, quickStartsError];
+  return [bestMatchQuickStarts as QuickStart[], quickStartsLoaded, quickStartsError];


The getBestMatch function's null return type is forcing us to use a type assertion as. While we could remove the null return type to fix this, I'm unsure about the potential impact on other parts of the codebase where it's used.
IMO, I think it's better to filter out the null values after the map function, since the useQuickStarts hook doesn't have a corresponding null return type. Something like:

.filter((quickStart) => quickStart !== null);

cajieh · 2025-08-20T17:33:32Z

...end/packages/console-app/src/components/resource-quota/AppliedClusterResourceQuotaCharts.tsx

-    const nsUsed = nsQuotas.status.used?.[resourceName];
-    const clusterUsage = getUsedPercentage(clusterHard, clusterUsed);
+    const clusterHard = appliedClusterResourceQuota.status?.total?.hard?.[resourceName || ''];
+    const clusterUsed = appliedClusterResourceQuota.status?.total?.used?.[resourceName || ''];


I am unsure of the need for empty strings '' here. Was this done to fix other type issue? I ask because removing it doesn't seem to cause an error when "strictNullChecks": true is enabled.

yeah i tried another fix.

const clusterHard = clusterResourceQuota.status?.total?.hard?.[resourceName]; const clusterUsed = clusterResourceQuota.status?.total?.used?.[resourceName]; const { label, percent } = getLabelAndUsage({ resourceName, used: clusterUsed ?? '', hard: clusterHard ?? '',

cajieh · 2025-08-20T17:34:28Z

frontend/packages/console-app/src/components/resource-quota/ClusterResourceQuotaCharts.tsx

-    const clusterHard = clusterResourceQuota.status.total.hard[resourceName];
-    const clusterUsed = clusterResourceQuota.status.total.used?.[resourceName];
+    const clusterHard = clusterResourceQuota.status?.total?.hard?.[resourceName || ''];
+    const clusterUsed = clusterResourceQuota.status?.total?.used?.[resourceName || ''];


Same as above.

cajieh · 2025-08-20T17:35:00Z

frontend/packages/console-app/src/components/resource-quota/ResourceQuotaCharts.tsx

-    const hard = resourceQuota.status.hard[resourceName];
-    const used = resourceQuota.status.used?.[resourceName];
+    const hard = resourceQuota.status?.hard?.[resourceName || ''];
+    const used = resourceQuota.status?.used?.[resourceName || ''];


Same as above.

cajieh · 2025-08-20T18:10:02Z

frontend/packages/console-app/src/components/tour/GuidedTour.tsx

@@ -32,7 +32,7 @@ const GuidedTour: React.FC = () => {
        backButtonText={t('console-app~Back')}
      />
    );
-  const step = steps[stepNumber - 1];
+  const step = steps?.[(stepNumber || 0) - 1];


To prevent potential issues with negative values, could we assign a default value to stepNumber before this block? This would ensure the if (startTour || stepNumber === 0) { check runs first.

cajieh · 2025-08-20T18:14:07Z

frontend/packages/console-app/src/components/tour/TourStepComponent.tsx

        onClick: () => {
          onBack && onBack();
        },
      }}
      step={step}
    >
-      {showStepBadge ? <StepBadge stepNumber={step} totalSteps={totalSteps} /> : null}
+      {showStepBadge ? <StepBadge stepNumber={step || 0} totalSteps={totalSteps || 0} /> : null}


The default alternative mentioned above might work here as well.

krishagarwal278 · 2025-08-21T13:33:17Z

/retest

yanpzhan · 2025-08-21T13:34:04Z

Local tsc check passed against the files in pr code.
But when access quickstart page '/quickstart' on cluster launched against the pr, the page loaded very slowly, once the quickstarts are loaded, clicking on one quickstart item has no response.
@krishagarwal278 Could you help to check if code change related quickstarts affects the page usage?

cajieh

The work looks good👍, although I added a nit.
Thank you.😁
/lgtm

cajieh · 2025-08-25T13:02:32Z

frontend/packages/console-app/src/components/tour/TourStepComponent.tsx

        onClick: () => {
          onBack && onBack();
        },
      }}
      step={step}
    >
-      {showStepBadge ? <StepBadge stepNumber={step} totalSteps={totalSteps} /> : null}
+      {showStepBadge ? <StepBadge stepNumber={step ?? 0} totalSteps={totalSteps ?? 0} /> : null}


nit: The props default values might be cleaner than Nullish Coalescing Operator.

cajieh · 2025-08-25T16:54:20Z

@krishagarwal278 Could you squash the commits?

krishagarwal278 · 2025-08-25T17:30:13Z

/label tide/merge-method-squash

cajieh · 2025-08-25T17:46:30Z

/lgtm

krishagarwal278 · 2025-08-25T21:44:39Z

/label acknowledge-critical-fixes-only

krishagarwal278 · 2025-08-25T22:58:49Z

/test frontend

yanpzhan · 2025-08-26T06:46:56Z

Checked the pr again, there is not regression issue now.
/label qe-approved

openshift-ci-robot · 2025-08-26T06:47:02Z

@krishagarwal278: This pull request references CONSOLE-4697 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the sub-task to target the "4.20.0" version, but no target version was set.

In response to this:

Fixed SNC errors in storage, resource-quota, quick-starts, tour, tab, vpa

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci-robot · 2025-08-26T07:28:49Z

/retest-required

Remaining retests: 0 against base HEAD 2151cf2 and 2 for PR HEAD 94b544c in total

openshift-ci-robot · 2025-08-26T11:13:26Z

/retest-required

Remaining retests: 0 against base HEAD 2151cf2 and 2 for PR HEAD 94b544c in total

openshift-ci-robot · 2025-08-26T16:27:18Z

/retest-required

Remaining retests: 0 against base HEAD 2151cf2 and 2 for PR HEAD 94b544c in total

krishagarwal278 · 2025-08-26T17:58:40Z

/retest

cajieh · 2025-08-26T20:14:18Z

/lgtm

openshift-ci · 2025-08-26T20:15:42Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cajieh, krishagarwal278

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~frontend/OWNERS~~ [cajieh]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot · 2025-08-26T22:31:08Z

/retest-required

Remaining retests: 0 against base HEAD 2151cf2 and 2 for PR HEAD d3f5043 in total

openshift-ci-robot · 2025-08-27T10:28:48Z

/retest-required

Remaining retests: 0 against base HEAD 2151cf2 and 2 for PR HEAD d3f5043 in total

openshift-ci-robot · 2025-08-27T19:23:45Z

/retest-required

Remaining retests: 0 against base HEAD 9592eef and 2 for PR HEAD d3f5043 in total

openshift-ci-robot · 2025-08-28T00:23:42Z

/retest-required

Remaining retests: 0 against base HEAD 963a204 and 1 for PR HEAD d3f5043 in total

openshift-ci-robot · 2025-08-28T05:23:49Z

/retest-required

Remaining retests: 0 against base HEAD 02f0f4a and 0 for PR HEAD d3f5043 in total

openshift-ci · 2025-08-28T09:24:26Z

@krishagarwal278: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/frontend	`d3f5043`	link	true	`/test frontend`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

openshift-ci-robot · 2025-08-28T10:23:58Z

/hold

Revision d3f5043 was retested 3 times: holding

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Aug 19, 2025

openshift-ci bot requested review from jhadvig and TheRealJon August 19, 2025 16:03

openshift-ci bot added the component/core Related to console core functionality label Aug 19, 2025

openshift-ci bot added docs-approved Signifies that Docs has signed off on this PR px-approved Signifies that Product Support has signed off on this PR labels Aug 19, 2025

cajieh reviewed Aug 20, 2025

View reviewed changes

krishagarwal278 force-pushed the CONSOLE-4697 branch from 96f9a02 to af7ce6c Compare August 21, 2025 18:03

Fixed SNC errors and addressed PR reviews

328ac59

krishagarwal278 force-pushed the CONSOLE-4697 branch from af7ce6c to 328ac59 Compare August 21, 2025 19:16

cajieh reviewed Aug 25, 2025

View reviewed changes

openshift-ci bot assigned cajieh Aug 25, 2025

openshift-ci bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 25, 2025

Fixed a small nit

94b544c

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 25, 2025

openshift-ci bot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Aug 25, 2025

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 25, 2025

openshift-ci bot added the acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. label Aug 25, 2025

openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Aug 26, 2025

Fixed frontend test failure

d3f5043

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 26, 2025

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 26, 2025

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 28, 2025

CONSOLE-4697: Enable strickNullCheck for /console-app/components/resource-quota | quick-starts | tour | tabs | vpa | storage #15418

Are you sure you want to change the base?

CONSOLE-4697: Enable strickNullCheck for /console-app/components/resource-quota | quick-starts | tour | tabs | vpa | storage #15418

Uh oh!

Conversation

krishagarwal278 commented Aug 19, 2025

Uh oh!

openshift-ci-robot commented Aug 19, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

krishagarwal278 commented Aug 19, 2025

Uh oh!

krishagarwal278 commented Aug 20, 2025

Uh oh!

cajieh left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krishagarwal278 commented Aug 21, 2025

Uh oh!

yanpzhan commented Aug 21, 2025

Uh oh!

cajieh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cajieh commented Aug 25, 2025

Uh oh!

krishagarwal278 commented Aug 25, 2025

Uh oh!

cajieh commented Aug 25, 2025

Uh oh!

krishagarwal278 commented Aug 25, 2025

Uh oh!

krishagarwal278 commented Aug 25, 2025

Uh oh!

yanpzhan commented Aug 26, 2025

Uh oh!

openshift-ci-robot commented Aug 26, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Aug 26, 2025

Uh oh!

openshift-ci-robot commented Aug 26, 2025

Uh oh!

openshift-ci-robot commented Aug 26, 2025

Uh oh!

krishagarwal278 commented Aug 26, 2025

Uh oh!

cajieh commented Aug 26, 2025

Uh oh!

openshift-ci bot commented Aug 26, 2025

Uh oh!

openshift-ci-robot commented Aug 26, 2025

Uh oh!

openshift-ci-robot commented Aug 27, 2025

Uh oh!

openshift-ci-robot commented Aug 27, 2025

Uh oh!

openshift-ci-robot commented Aug 28, 2025

Uh oh!

openshift-ci-robot commented Aug 19, 2025 •

edited by openshift-ci bot

Loading

cajieh left a comment •

edited

Loading

openshift-ci-robot commented Aug 26, 2025 •

edited by openshift-ci bot

Loading