Skip to content

RHIDP-12113: Support customization aggregated homepage scorecard card#2376

Closed
imykhno wants to merge 11 commits intoredhat-developer:mainfrom
imykhno:scorecard-customization-aggregated-KPI
Closed

RHIDP-12113: Support customization aggregated homepage scorecard card#2376
imykhno wants to merge 11 commits intoredhat-developer:mainfrom
imykhno:scorecard-customization-aggregated-KPI

Conversation

@imykhno
Copy link
Contributor

@imykhno imykhno commented Feb 23, 2026

User description

Hey, I just made a Pull Request!

This PR enables the customization of title and descriptions for aggregated KPIs within the scorecard plugin. This functionality allows Engineering and Product Managers to provide company-specific information, ensuring metrics are easily understood by end-users.

The description logic operates as follows:

  • Non-customized Aggregated KPI cards: These use the default title and description. Localisation/translation remains functional for all supported languages.
  • Customized Aggregated KPI cards: These use the title and description provided by the system administrator. Please note that translation is disabled for customised cards.

The app-config.yaml for the scorecard plugin has been expanded to include a homepage section. Here, system administrators can define custom values for each provider.

Was expanded app-config.yaml for the scorecard plugin. Added homepage section where system administrator can provide customized title and description for each providers.

The example of customization at the app-config.yaml file:

scorecard:
  plugins:
    jira:
      open_issues:
        aggregations:
          title: 'My Jira issues'
          description: 'Custom description for Jira open issues.'

Additionally, the response object for GET /metrics/:metricId/catalog/aggregations has been updated. I have added a customized flag to the metadata to indicate whether the fields have been modified, preventing unnecessary translation attempts.

New logic implemented for:

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or Updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)

How to test?

1. Aggregated KPI works as expected without customization

  • Configure scorecard aggregated KPI at the homepage;
  • Confirm the default titles and descriptions displayed as expected (ref for Eng);
  • Confirm the translation works as expected for supported languages.

2. Aggregated KPI works as expected when customized names and descriptions were provided

  • Configure scorecard aggregated KPI with changed title and description;
  • Confirm the new title and description presented on the homepage;
  • Confirm the translation no longer affect the customized cards.

3. Error will be thrown when customized only names OR descriptions

  • Configure scorecard aggregated KPI with changed title OR description;
  • Confirm the 400 InputError will be displayed on the UI.

PR Type

Enhancement


Description

  • Enable customization of aggregated KPI card titles and descriptions via app-config.yaml

  • Add customized flag to API response metadata to disable translations for custom values

  • Validate that both title and description are provided together, throwing InputError if incomplete

  • Update frontend to render custom titles/descriptions when customization is configured


Diagram Walkthrough

flowchart LR
  A["app-config.yaml<br/>homepage.aggregatedMetric"] -->|"metricId + config"| B["getAggregatedMetricCustomization"]
  B -->|"validation & trimming"| C["MetricCustomization<br/>isCustomized flag"]
  C -->|"passed to mapper"| D["AggregatedMetricResult<br/>with customized metadata"]
  D -->|"API response"| E["Frontend Component<br/>renders custom or default"]
Loading

File Walkthrough

Relevant files
Configuration changes
1 files
config.d.ts
Add homepage aggregatedMetric config schema                           
+10/-0   
Enhancement
6 files
plugin.ts
Pass config to router initialization                                         
+1/-0     
mappers.ts
Add customized flag to AggregatedMetricResult                       
+2/-0     
router.ts
Integrate customization logic into aggregation endpoint   
+19/-1   
getAggregatedMetricCustomization.ts
New utility to extract and validate metric customization 
+64/-0   
Metric.ts
Add optional customized flag to AggregatedMetricResult     
+1/-0     
ScorecardHomepageCard.tsx
Render custom titles/descriptions when customized flag set
+11/-0   
Tests
3 files
getAggregatedMetricCustomization.test.ts
Comprehensive tests for customization validation logic     
+176/-0 
router.test.ts
Add tests for customized metrics and error handling           
+69/-1   
ScorecardHomepageSection.test.tsx
Test custom title and description rendering                           
+24/-0   
Documentation
4 files
README.md
Document aggregated KPI customization feature                       
+26/-0   
app-config.local.EXAMPLE.yaml
Add example configuration for customization                           
+6/-0     
report.api.md
Update API report with customized field                                   
+1/-0     
heavy-hairs-press.md
Add changeset for feature release                                               
+7/-0     

… "descriptions" for homepage aggregated KPIs

Signed-off-by: Ihor Mykhno <imykhno@redhat.com>
@rhdh-gh-app
Copy link

rhdh-gh-app bot commented Feb 23, 2026

Important

This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior.

Changed Packages

Package Name Package Path Changeset Bump Current Version
app workspaces/scorecard/packages/app none v0.0.0
@red-hat-developer-hub/backstage-plugin-scorecard-backend workspaces/scorecard/plugins/scorecard-backend minor v2.3.5
@red-hat-developer-hub/backstage-plugin-scorecard-common workspaces/scorecard/plugins/scorecard-common minor v2.3.5
@red-hat-developer-hub/backstage-plugin-scorecard workspaces/scorecard/plugins/scorecard minor v2.3.5

@rhdh-qodo-merge
Copy link

rhdh-qodo-merge bot commented Feb 23, 2026

PR Compliance Guide 🔍

(Compliance updated until commit cc26437)

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
buildConfig Component

Description:
buildConfig reimplements the same pattern as newMockRootConfig (wrapping
mockServices.rootConfig with a scorecard.plugins[provider][metricName] structure) but in a
more generic way. Consider refactoring to reuse/extend newMockRootConfig (e.g., accept
provider, metricName, and plugin config overrides) or introduce a shared helper like
newMockPluginRootConfig(provider, metricName, pluginConfig) used by both.

PR Code:
getAggregatedMetricCustomization.test.ts [25-38]

function buildConfig(pluginConfig: any, metricId: string) {
  const [provider, metricName] = metricId.split('.');
  return mockServices.rootConfig({
    data: {
      scorecard: {
        plugins: {
          [provider]: {
            [metricName]: pluginConfig,
          },
        },
      },
    },
  });
}

Codebase Context Code:
redhat-developer/rhdh-plugins/workspaces/scorecard/plugins/scorecard-backend-module-jira/fixtures/testUtils.ts [62-90]

export function newMockRootConfig({
 thresholds,
 options,
 jiraConfig,
}: NewMockRootConfigProps = {}): Config {
 const jira = {
   baseUrl: 'https://example.com/api',
   token: 'Fds31dsF32',
   product: 'cloud',
   proxyPath: '/jira/api',
   ...jiraConfig,
 };

 return mockServices.rootConfig({
   data: {
     jira,
     scorecard: {
       plugins: {
         jira: {
           open_issues: {


... (clipped 9 lines)
Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

🔴
Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: 🏷️
Leaky error details: The client rethrows backend response bodies (errorText) inside user-facing errors, which
can expose internal implementation details if the backend returns stack traces or
sensitive diagnostics.

Referred Code
try {
  const response = await this.fetchApi.fetch(url.toString());

  if (!response.ok) {
    const errorText = await response.text();
    throw new Error(
      `Failed to fetch metric: ${response.status} ${response.statusText}. ${errorText}`,
    );
  }

  const data = await response.json();

  if (
    !data ||
    Array.isArray(data) ||
    typeof data !== 'object' ||
    !('metrics' in data) ||
    !Array.isArray((data as any).metrics)
  ) {
    throw new TypeError('Invalid response format from metrics API');
  }


 ... (clipped 8 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: 🏷️
Unhandled config error: The new /metrics handler calls getAggregatedMetricCustomization for each metric without a
local catch, so an unexpected InputError (e.g., malformed metric IDs) could fail the whole
endpoint instead of gracefully degrading per-metric.

Referred Code
const getMetrics = () => {
  if (metricIds) {
    return metricProvidersRegistry.listMetrics(
      parseCommaSeparatedString(metricIds),
    );
  }

  if (datasource) {
    return metricProvidersRegistry.listMetricsByDatasource(datasource);
  }

  return metricProvidersRegistry.listMetrics();
};

const metrics = getMetrics().map(metric => {
  const { isCustomized, title, description } =
    getAggregatedMetricCustomization(metric.id, { config });

  return {
    ...metric,
    isCustomized,


 ... (clipped 8 lines)

Learn more about managing compliance generic rules or creating your own custom rules

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Previous compliance checks

Compliance check up to commit 0f39f1a
Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
buildConfig Component

Description:
buildConfig reimplements the same mockServices.rootConfig({ data: { scorecard: { plugins:
... }}}) test-config construction pattern as newMockRootConfig, just generalized by
splitting metricId into provider/metric keys. Consider refactoring to a shared helper
(e.g., a generic newMockPluginRootConfig(providerId, metricName, pluginConfig) or extend
newMockRootConfig to accept arbitrary provider/metric) and reuse it in these tests.

PR Code:
getAggregatedMetricCustomization.test.ts [25-38]

function buildConfig(pluginConfig: any, metricId: string) {
  const [provider, metricName] = metricId.split('.');
  return mockServices.rootConfig({
    data: {
      scorecard: {
        plugins: {
          [provider]: {
            [metricName]: pluginConfig,
          },
        },
      },
    },
  });
}

Codebase Context Code:
redhat-developer/rhdh-plugins/workspaces/scorecard/plugins/scorecard-backend-module-jira/fixtures/testUtils.ts [62-90]

export function newMockRootConfig({
 thresholds,
 options,
 jiraConfig,
}: NewMockRootConfigProps = {}): Config {
 const jira = {
   baseUrl: 'https://example.com/api',
   token: 'Fds31dsF32',
   product: 'cloud',
   proxyPath: '/jira/api',
   ...jiraConfig,
 };

 return mockServices.rootConfig({
   data: {
     jira,
     scorecard: {
       plugins: {
         jira: {
           open_issues: {


... (clipped 9 lines)
Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

🔴
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Misleading error message: The UI reports Multiple metrics found for any case where metrics.length !== 1 (including
zero results), which is misleading and weakens edge-case handling.

Referred Code
if (metricsError || metrics.length !== 1) {
  return (
    <ResponseErrorPanel
      error={metricsError || new Error('Multiple metrics found')}
    />

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
Error text exposure: The new getMetrics client method embeds raw backend errorText into the thrown Error
message, which may be rendered to end-users and could leak internal details depending on
backend responses and UI error presentation.

Referred Code
if (!response.ok) {
  const errorText = await response.text();
  throw new Error(
    `Failed to fetch metric: ${response.status} ${response.statusText}. ${errorText}`,
  );

Learn more about managing compliance generic rules or creating your own custom rules

Compliance check up to commit e373846
Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
🔴
mockMetricsResponse Component

Description:
mockMetricsResponse duplicates the same Playwright page.route(...).fulfill({ status,
contentType: 'application/json', body: JSON.stringify(...) }) pattern twice,
matching mockAggregatedScorecardResponse almost exactly (only route constants and payload
names differ). Refactor to reuse the existing mockApiResponse helper (Similar Content #3)
for each route, or generalize the existing two-route mock helper to accept route constants
for GitHub/Jira.

PR Code:
apiUtils.ts [84-105]

export async function mockMetricsResponse(
  page: Page,
  githubResponse: object,
  jiraResponse: object,
  status = 200,
) {
  await page.route(GITHUB_METRICS_API_ROUTE, async route => {
    await route.fulfill({
      status,
      contentType: 'application/json',
      body: JSON.stringify(githubResponse),
    });
  });

  await page.route(JIRA_METRICS_API_ROUTE, async route => {
    await route.fulfill({
      status,
      contentType: 'application/json',
      body: JSON.stringify(jiraResponse),
    });
  });


 ... (clipped 1 lines)

Codebase Context Code:
redhat-developer/rhdh-plugins/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.test.ts [480-501]

export async function mockAggregatedScorecardResponse(
 page: Page,
 githubResponse: object,
 jiraResponse: object,
 status = 200,
) {
 await page.route(GITHUB_AGGREGATION_ROUTE, async route => {
   await route.fulfill({
     status,
     contentType: 'application/json',
     body: JSON.stringify(githubResponse),
   });
 });

 await page.route(JIRA_AGGREGATION_ROUTE, async route => {
   await route.fulfill({
     status,
     contentType: 'application/json',
     body: JSON.stringify(jiraResponse),
   });


... (clipped 2 lines)
buildConfig Component

Description:
buildConfig reimplements the same mockServices.rootConfig({ data: { scorecard: { plugins:
... }}}) construction pattern as newMockRootConfig, but in a more generic way by deriving
[provider][metricName] from metricId. Consider extracting a shared
buildPluginMetricConfig(metricId, pluginConfig, extraRootData?) test utility that both
newMockRootConfig and this new buildConfig can call to avoid repeating nested config
scaffolding.

PR Code:
getAggregatedMetricCustomization.test.ts [25-38]

function buildConfig(pluginConfig: any, metricId: string) {
  const [provider, metricName] = metricId.split('.');
  return mockServices.rootConfig({
    data: {
      scorecard: {
        plugins: {
          [provider]: {
            [metricName]: pluginConfig,
          },
        },
      },
    },
  });
}

Codebase Context Code:
redhat-developer/rhdh-plugins/workspaces/scorecard/plugins/scorecard-backend-module-jira/fixtures/testUtils.ts [62-90]

export function newMockRootConfig({
 thresholds,
 options,
 jiraConfig,
}: NewMockRootConfigProps = {}): Config {
 const jira = {
   baseUrl: 'https://example.com/api',
   token: 'Fds31dsF32',
   product: 'cloud',
   proxyPath: '/jira/api',
   ...jiraConfig,
 };

 return mockServices.rootConfig({
   data: {
     jira,
     scorecard: {
       plugins: {
         jira: {
           open_issues: {


... (clipped 9 lines)
Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

🔴
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Swallowed exceptions: The new GET /metrics handler catches all errors from getAggregatedMetricCustomization and
silently returns the uncustomized metric, preventing observability and making
configuration/format errors hard to detect or troubleshoot.

Referred Code
const metrics = getMetrics().map(metric => {
  try {
    const { isCustomized, title, description } =
      getAggregatedMetricCustomization(metric.id, { config });

    return {
      ...metric,
      isCustomized,
      ...(isCustomized ? { title, description } : {}),
    };
  } catch (error) {
    // If metric ID format or config invalid, keep original metric without customization
    return metric;
  }

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
Error details leak: The frontend API client throws errors that append raw backend response.text() content
(potentially including internal details) into the Error message, which may be shown to end
users depending on UI error rendering.

Referred Code
  const errorText = await response.text();
  throw new Error(
    `Failed to fetch aggregated scorecards: ${response.status} ${response.statusText}. ${errorText}`,
  );
}

const data = await response.json();

if (
  !data ||
  Array.isArray(data) ||
  typeof data !== 'object' ||
  !('result' in data) ||
  !('metadata' in data) ||
  !('id' in data) ||
  !('status' in data)
) {
  throw new TypeError(
    'Invalid response format from aggregated scorecard API',
  );
}


 ... (clipped 41 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Compliance check up to commit 47a742b
Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
🟢
No codebase code duplication found New Components Detected (Top 5):
- it: should return aggregated metrics with custom title and description when aggregated metric is customized
- buildConfig
- describe: getAggregatedMetricCustomization
- it: should return isCustomized false when aggregatedMetric is not configured
- it: should return customized object when title and description are set
Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

🔴
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: 🏷️
Stale async fetch: The useAsync dependency list omits metricId (and t), so the hook may not refetch when the
metric ID changes and can show stale or incorrect data.

Referred Code
const { error, loading, value } = useAsync(async () => {
  try {
    const metrics = await scorecardApi.getMetrics({ metricIds: [metricId] });

    if (!metrics || Array.isArray(metrics) || typeof metrics !== 'object') {
      throw new Error(t('errors.invalidApiResponse'));
    }

    return metrics;
  } catch (err) {
    if (err instanceof Error) {
      throw err;
    }
    throw new Error(
      t('errors.fetchError' as any, {
        error: String(err),
      }),
    );
  }
}, [scorecardApi]);

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: 🏷️
Detailed config path: The thrown InputError messages include internal configuration paths (e.g.,
scorecard.plugins.<provider>.<metricName>...) which may be surfaced to end
users depending on error handling and could disclose internal structure.

Referred Code
  throw new InputError(
    `${configPath} requires both title and description when customizing aggregated KPI`,
  );
}

const trimmedTitle = toTrimmedString(title);
const trimmedDescription = toTrimmedString(description);

if (!trimmedTitle || !trimmedDescription) {
  throw new InputError(
    `${configPath} requires both title and description to be non-empty strings`,
  );
}

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: 🏷️
Unhandled metricId format: GET /metrics calls getAggregatedMetricCustomization(metric.id, { config }) for every
metric and will throw InputError if any metric.id is not in provider.metricName format,
potentially breaking the entire endpoint instead of skipping/handling unexpected IDs.

Referred Code
const getMetrics = () => {
  if (metricIds) {
    return metricProvidersRegistry.listMetrics(
      parseCommaSeparatedString(metricIds),
    );
  }

  if (datasource) {
    return metricProvidersRegistry.listMetricsByDatasource(datasource);
  }

  return metricProvidersRegistry.listMetrics();
};

return res.json({
  metrics: getMetrics().map(metric => {
    const { isCustomized, title, description } =
      getAggregatedMetricCustomization(metric.id, { config });

    return isCustomized
      ? { ...metric, isCustomized, title, description }


 ... (clipped 3 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Compliance check up to commit 712e1d5
Security Compliance
Config-driven XSS

Description: Possible stored XSS/content-injection risk because administrator-supplied
title/description from configuration are returned by the backend and rendered in the UI
when aggregatedScorecard.metadata.customized is true; while React typically escapes
strings, this becomes exploitable if ScorecardHomepageCardComponent (or downstream
components) ever render these fields as HTML (e.g., via dangerouslySetInnerHTML) or pass
them into an HTML-capable markdown renderer without sanitization.
ScorecardHomepageCard.tsx [64-73]

Referred Code
if (aggregatedScorecard.metadata.customized) {
  return (
    <ScorecardHomepageCardComponent
      key={aggregatedScorecard.id}
      cardTitle={aggregatedScorecard.metadata.title}
      description={aggregatedScorecard.metadata.description}
      scorecard={aggregatedScorecard}
    />
  );
}
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
buildConfig Component

Description:
buildConfig reimplements the same “build a mockServices.rootConfig with
scorecard.plugins.<provider>.<metric>” pattern already covered by
newMockRootConfig. Consider extending newMockRootConfig (or introducing a shared helper)
to accept a generic { metricId, pluginConfig } input so tests don’t duplicate
config-shaping logic.

PR Code:
getAggregatedMetricCustomization.test.ts [25-38]

function buildConfig(pluginConfig: any, metricId: string) {
  const [provider, metricName] = metricId.split('.');
  return mockServices.rootConfig({
    data: {
      scorecard: {
        plugins: {
          [provider]: {
            [metricName]: pluginConfig,
          },
        },
      },
    },
  });
}

Codebase Context Code:
redhat-developer/rhdh-plugins/workspaces/scorecard/plugins/scorecard-backend-module-jira/fixtures/testUtils.ts [62-90]

export function newMockRootConfig({
 thresholds,
 options,
 jiraConfig,
}: NewMockRootConfigProps = {}): Config {
 const jira = {
   baseUrl: 'https://example.com/api',
   token: 'Fds31dsF32',
   product: 'cloud',
   proxyPath: '/jira/api',
   ...jiraConfig,
 };

 return mockServices.rootConfig({
   data: {
     jira,
     scorecard: {
       plugins: {
         jira: {
           open_issues: {


... (clipped 9 lines)
Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
Verbose user errors: The thrown InputError messages include internal configuration paths (e.g.,
scorecard.plugins.<metricId>.homepage.aggregatedMetric...) that may be surfaced to
end-users via the API error response.

Referred Code
  throw new InputError(
    `scorecard.plugins.${metricId}.homepage.aggregatedMetric requires both title and description when customizing aggregated KPI`,
  );
}

const trimmedTitle = toTrimmedString(title);
const trimmedDescription = toTrimmedString(description);

if (!trimmedTitle || !trimmedDescription) {
  throw new InputError(
    `scorecard.plugins.${metricId}.homepage.aggregatedMetric requires both title and description to be non-empty strings`,
  );

Learn more about managing compliance generic rules or creating your own custom rules

@rhdh-qodo-merge
Copy link

rhdh-qodo-merge bot commented Feb 23, 2026

PR Code Suggestions ✨

Latest suggestions up to cc26437

CategorySuggestion                                                                                                                                    Impact
Incremental [*]
Validate inputs before API calls

Add a check in the useMetric hook to ensure metricId is a non-empty string
before making an API call, preventing potential misleading errors.

workspaces/scorecard/plugins/scorecard/src/hooks/useMetric.tsx [34-42]

 const { error, loading, value } = useAsync(async () => {
   try {
+    if (!metricId || metricId.trim() === '') {
+      throw new Error(t('errors.noMetricsFound'));
+    }
+
     const { metrics } = await scorecardApi.getMetrics({
       metricIds: [metricId],
     });
 
     if (metrics.length === 0) {
       throw new Error(t('errors.noMetricsFound'));
     }
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion improves the robustness of the useMetric hook by adding input validation for metricId, preventing misleading errors and unnecessary API calls with invalid input.

Medium
Validate and normalize API responses

Add validation and normalization for the isCustomized field in the
getAggregatedScorecard API response to ensure it is always a boolean, improving
client-side robustness.

workspaces/scorecard/plugins/scorecard/src/api/index.ts [165-179]

 if (
   !data ||
   Array.isArray(data) ||
   typeof data !== 'object' ||
   !('result' in data) ||
   !('metadata' in data) ||
   !('id' in data) ||
-  !('status' in data)
+  !('status' in data) ||
+  typeof (data as any).metadata !== 'object' ||
+  (data as any).metadata === null
 ) {
   throw new TypeError(
     'Invalid response format from aggregated scorecard API',
   );
 }
 
-return data as AggregatedMetricResult;
+const normalized = {
+  ...(data as any),
+  metadata: {
+    ...(data as any).metadata,
+    isCustomized: Boolean((data as any).metadata.isCustomized),
+  },
+};
 
+return normalized as AggregatedMetricResult;
+
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies a potential robustness issue where the client does not validate the new isCustomized field, and proposes a sound normalization strategy to prevent runtime errors from malformed API responses.

Low
Possible issue
Prevent error-on-error masking

In ErrorStatePanel, avoid making a secondary /metrics call for permission or
authentication errors to prevent masking the original error. Instead, handle
these errors first and derive the card title and description from translations.

workspaces/scorecard/plugins/scorecard/src/components/ScorecardHomepageSection/ErrorStatePanel.tsx [24-44]

 export const ErrorStatePanel = ({
   error,
   metricId,
 }: {
   error: Error;
   metricId: string;
 }) => {
   const { t } = useTranslation();
 
+  const isMissingPermission = error.message?.includes('NotAllowedError');
+  const isAuthenticationError = error.message?.includes('AuthenticationError');
+
+  // For auth/permission failures, fetching metric details is likely to fail too;
+  // use translations directly to avoid masking the original error.
+  if (isMissingPermission || isAuthenticationError) {
+    const titleKey = `metric.${metricId}.title`;
+    const descriptionKey = `metric.${metricId}.description`;
+
+    const translatedTitle = t(titleKey as any, {});
+    const translatedDescription = t(descriptionKey as any, {});
+
+    const cardTitle = translatedTitle !== titleKey ? translatedTitle : metricId;
+    const cardDescription =
+      translatedDescription !== descriptionKey ? translatedDescription : '';
+
+    return (
+      <EmptyStatePanel
+        label={
+          isMissingPermission
+            ? t('errors.missingPermission')
+            : t('errors.authenticationError')
+        }
+        cardTitle={cardTitle}
+        cardDescription={cardDescription}
+        tooltipContent={
+          isMissingPermission
+            ? t('errors.missingPermissionMessage')
+            : t('errors.authenticationErrorMessage')
+        }
+      />
+    );
+  }
+
   const { metric, loading, error: metricError } = useMetric({ metricId });
-
   const { title: cardTitle, description: cardDescription } =
     useMetricDisplayLabels(metric);
 
   if (loading) {
     return <CardLoading />;
   }
 
   if (metricError) {
     return <ResponseErrorPanel error={metricError} />;
   }
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a bug where a secondary API call (useMetric) in an error handler can fail for the same reason as the original call, masking the intended error message and degrading the user experience.

Medium
Stabilize hook translation tests

In the test for useMetricDisplayLabels, mock the useTranslation hook to provide
deterministic translations. This will ensure the test is isolated and does not
depend on a global i18n setup.

workspaces/scorecard/plugins/scorecard/src/hooks/tests/useMetricDisplayLabels.test.tsx [1-62]

 import { renderHook } from '@testing-library/react';
 import { useMetricDisplayLabels } from '../useMetricDisplayLabels';
 import { MetricsDetails } from '@red-hat-developer-hub/backstage-plugin-scorecard-common';
+
+jest.mock('../../hooks/useTranslation', () => ({
+  useTranslation: () => ({
+    t: (key: string) => {
+      if (key === 'metric.jira.open_issues.title') return 'Jira open blocking tickets';
+      if (key === 'metric.jira.open_issues.description') {
+        return 'Highlights the number of critical, blocking issues that are currently open in Jira.';
+      }
+      return key;
+    },
+  }),
+}));
 
 describe('useMetricDisplayLabels', () => {
   it('should return empty title and description when metric is undefined', () => {
     const { result } = renderHook(() => useMetricDisplayLabels());
 
     expect(result.current).toEqual({ title: '', description: '' });
   });
 
   it('should return title and description as-is when metric is customized', () => {
     const metric = {
       id: 'github.open_prs',
       title: 'Admin custom title',
       description: 'Admin custom description.',
       type: 'number',
       isCustomized: true,
     } as MetricsDetails;
 
     const { result } = renderHook(() => useMetricDisplayLabels(metric));
 
     expect(result.current).toEqual({
       title: 'Admin custom title',
       description: 'Admin custom description.',
     });
   });
 
   it('should use metadata title and description when not customized', () => {
     const metric = {
       id: 'jira.open_issues',
       title: 'Jira open issues',
       description: 'Jira open issues description',
       type: 'number',
       isCustomized: false,
     } as MetricsDetails;
 
     const { result } = renderHook(() => useMetricDisplayLabels(metric));
 
     expect(result.current).toEqual({
       title: 'Jira open blocking tickets',
       description:
         'Highlights the number of critical, blocking issues that are currently open in Jira.',
     });
   });
 });

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out that the test for the useMetricDisplayLabels hook is not isolated and depends on an external translation context, which can lead to flaky or incorrect test results. Mocking useTranslation makes the test robust and self-contained.

Medium
Fix dev metrics mock data

In the getMetrics mock, filter by the correct metric ID (github.open_prs instead
of github.open_issues) and improve type safety by mapping the data to the
MetricsDetails shape instead of casting through unknown.

workspaces/scorecard/plugins/scorecard/dev/index.tsx [69-77]

 async getMetrics(_options?: MetricsOptions): Promise<MetricsResponse> {
-  return {
-    metrics: [
-      ...mockScorecardSuccessData.filter(
-        metric => metric.id === 'github.open_issues',
-      ),
-    ] as unknown as MetricsDetails[],
-  };
+  const metrics = mockScorecardSuccessData
+    .filter(metric => metric.id === 'github.open_prs')
+    .map(m => ({
+      id: m.id,
+      title: m.metadata.title,
+      description: m.metadata.description,
+      type: m.metadata.type,
+      isCustomized: false,
+    })) as MetricsDetails[];
+
+  return { metrics };
 }
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies a bug in the mock implementation where an incorrect metric ID (github.open_issues) would cause the dev environment to behave incorrectly, and also improves type safety by removing a cast through unknown.

Low
  • Update

Previous suggestions

✅ Suggestions up to commit 47a742b
CategorySuggestion                                                                                                                                    Impact
Incremental [*]
Build correct object for labels
Suggestion Impact:The commit replaces the unsafe cast of aggregatedScorecard.metadata with a newly constructed MetricsDetails-like object that includes the scorecard id and metadata fields, and passes that to useMetricDisplayLabels.

code diff:

+  const AggregatedMetricDetails = aggregatedScorecard
+    ? ({
+        id: aggregatedScorecard.id,
+        title: aggregatedScorecard.metadata.title,
+        description: aggregatedScorecard.metadata.description,
+        isCustomized: aggregatedScorecard.metadata.isCustomized,
+      } as MetricsDetails)
+    : undefined;
+
   const { title, description } = useMetricDisplayLabels(
-    aggregatedScorecard?.metadata as MetricsDetails,
+    AggregatedMetricDetails,
   );

Construct a proper MetricsDetails object for useMetricDisplayLabels by combining
aggregatedScorecard.id with aggregatedScorecard.metadata to prevent incorrect
translation key generation.

workspaces/scorecard/plugins/scorecard/src/components/ScorecardHomepageSection/ScorecardHomepageCard.tsx [33-35]

 const { title, description } = useMetricDisplayLabels(
-  aggregatedScorecard?.metadata as MetricsDetails,
+  aggregatedScorecard
+    ? ({
+        id: aggregatedScorecard.id,
+        ...aggregatedScorecard.metadata,
+      } as MetricsDetails)
+    : undefined,
 );

[Suggestion processed]

Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies that casting aggregatedScorecard.metadata to MetricsDetails is unsafe because it lacks the id property, which is crucial for the useMetricDisplayLabels hook to generate correct translation keys.

High
Always return customization flag
Suggestion Impact:The metrics mapping logic was changed to always spread isCustomized into the returned metric object and only conditionally include title/description, matching the suggested response shape. Additional refactoring and a try/catch were added around customization lookup.

code diff:

+    const metrics = getMetrics().map(metric => {
+      try {
         const { isCustomized, title, description } =
           getAggregatedMetricCustomization(metric.id, { config });
 
-        return isCustomized
-          ? { ...metric, isCustomized, title, description }
-          : metric;
-      }),
+        return {
+          ...metric,
+          isCustomized,
+          ...(isCustomized ? { title, description } : {}),
+        };
+      } catch (error) {
+        // If metric ID format or config invalid, keep original metric without customization
+        return metric;
+      }
+    });

Ensure the /metrics endpoint response always includes the isCustomized field for
each metric, even if it's false, to conform to the MetricsDetails type.

workspaces/scorecard/plugins/scorecard-backend/src/service/router.ts [125-134]

 return res.json({
   metrics: getMetrics().map(metric => {
     const { isCustomized, title, description } =
       getAggregatedMetricCustomization(metric.id, { config });
 
-    return isCustomized
-      ? { ...metric, isCustomized, title, description }
-      : metric;
+    return {
+      ...metric,
+      isCustomized,
+      ...(isCustomized ? { title, description } : {}),
+    };
   }),
 });

[Suggestion processed]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies that when a metric is not customized, the returned object lacks the isCustomized field, violating the MetricsDetails type contract and causing a potential frontend bug.

Medium
Possible issue
Refetch and validate hook responses
Suggestion Impact:The dependency array was updated to include metricId and t, addressing the stale data issue. However, the stricter API response validation was not implemented (the existing validation was removed instead).

code diff:

   const { error, loading, value } = useAsync(async () => {
     try {
-      const metrics = await scorecardApi.getMetrics({ metricIds: [metricId] });
-
-      if (!metrics || Array.isArray(metrics) || typeof metrics !== 'object') {
-        throw new Error(t('errors.invalidApiResponse'));
-      }
-
-      return metrics;
+      return await scorecardApi.getMetrics({ metricIds: [metricId] });
     } catch (err) {
       if (err instanceof Error) {
         throw err;
@@ -49,7 +43,7 @@
         }),
       );
     }
-  }, [scorecardApi]);
+  }, [scorecardApi, metricId, t]);

Add metricId and t to the useAsync dependency array to prevent stale data. Also,
add stricter validation for the API response to ensure it contains a metrics
array.

workspaces/scorecard/plugins/scorecard/src/hooks/useMetrics.tsx [33-52]

 const { error, loading, value } = useAsync(async () => {
   try {
-    const metrics = await scorecardApi.getMetrics({ metricIds: [metricId] });
+    const resp = await scorecardApi.getMetrics({ metricIds: [metricId] });
 
-    if (!metrics || Array.isArray(metrics) || typeof metrics !== 'object') {
+    if (
+      !resp ||
+      typeof resp !== 'object' ||
+      Array.isArray(resp) ||
+      !('metrics' in resp) ||
+      !Array.isArray((resp as any).metrics)
+    ) {
       throw new Error(t('errors.invalidApiResponse'));
     }
 
-    return metrics;
+    return resp;
   } catch (err) {
     if (err instanceof Error) {
       throw err;
     }
     throw new Error(
       t('errors.fetchError' as any, {
         error: String(err),
       }),
     );
   }
-}, [scorecardApi]);
+}, [scorecardApi, metricId, t]);

[Suggestion processed]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a critical bug in the useAsync dependency array which would cause stale data to be shown. It also improves response validation, making the hook more robust against unexpected API payloads.

Medium
Validate metrics API response shape
Suggestion Impact:The commit now parses the JSON into a variable, validates that it is an object with a metrics array, and throws a TypeError on invalid shapes before casting to MetricsResponse.

code diff:

-      return (await response.json()) as MetricsResponse;
+      const data = await response.json();
+
+      if (
+        !data ||
+        Array.isArray(data) ||
+        typeof data !== 'object' ||
+        !('metrics' in data) ||
+        !Array.isArray((data as any).metrics)
+      ) {
+        throw new TypeError('Invalid response format from metrics API');
+      }
+
+      return data as MetricsResponse;

Validate the response from the /metrics endpoint to ensure it contains a metrics
array before casting and returning it. This prevents potential UI crashes from
malformed API responses.

workspaces/scorecard/plugins/scorecard/src/api/index.ts [204]

-return (await response.json()) as MetricsResponse;
+const data = await response.json();
 
+if (
+  !data ||
+  typeof data !== 'object' ||
+  Array.isArray(data) ||
+  !('metrics' in data) ||
+  !Array.isArray((data as any).metrics)
+) {
+  throw new TypeError('Invalid response format from metrics API');
+}
+
+return data as MetricsResponse;
+

[Suggestion processed]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out that blindly casting the JSON response is unsafe. Adding validation to ensure the response contains a metrics array makes the API client more robust and prevents potential runtime errors in the UI.

Medium
✅ Suggestions up to commit 712e1d5
CategorySuggestion                                                                                                                                    Impact
General
Refactor to remove redundant code
Suggestion Impact:The route now extracts customization fields, builds a single `metricsDetails` object based on `isCustomized`, and makes one call to `AggregatedMetricMapper.toAggregatedMetricResult` (removing the previous duplicated branches).

code diff:

-    const aggregatedMetricCustomization = getAggregatedMetricCustomization(
-      metricId,
-      { config },
-    );
-
-    if (aggregatedMetricCustomization.isCustomized) {
-      const result = AggregatedMetricMapper.toAggregatedMetricResult(
-        { ...metric, ...aggregatedMetricCustomization },
+    const { isCustomized, title, description } =
+      getAggregatedMetricCustomization(metricId, { config });
+
+    const metricsDetails = isCustomized
+      ? { ...metric, title, description }
+      : metric;
+
+    return res.json(
+      AggregatedMetricMapper.toAggregatedMetricResult(
+        metricsDetails,
         aggregatedMetric,
-        aggregatedMetricCustomization.isCustomized,
-      );
-      return res.json(result);
-    }
-
-    return res.json(
-      AggregatedMetricMapper.toAggregatedMetricResult(metric, aggregatedMetric),
+        isCustomized,
+      ),
     );

Refactor the aggregated metric result creation to avoid redundant code by first
determining the metric details and then making a single function call.

workspaces/scorecard/plugins/scorecard-backend/src/service/router.ts [199-210]

-if (aggregatedMetricCustomization.isCustomized) {
-  const result = AggregatedMetricMapper.toAggregatedMetricResult(
-    { ...metric, ...aggregatedMetricCustomization },
-    aggregatedMetric,
-    aggregatedMetricCustomization.isCustomized,
-  );
-  return res.json(result);
-}
+const metricDetails = aggregatedMetricCustomization.isCustomized
+  ? { ...metric, ...aggregatedMetricCustomization }
+  : metric;
 
-return res.json(
-  AggregatedMetricMapper.toAggregatedMetricResult(metric, aggregatedMetric),
+const result = AggregatedMetricMapper.toAggregatedMetricResult(
+  metricDetails,
+  aggregatedMetric,
+  aggregatedMetricCustomization.isCustomized,
 );
 
+return res.json(result);
+

[Suggestion processed]

Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies redundant code and proposes a valid refactoring that improves readability and maintainability by applying the DRY principle.

Low
Improve encapsulation of a helper function
Suggestion Impact:The module-scope toTrimmedString helper was removed and redefined inside getAggregatedMetricCustomization, encapsulating the helper within the function.

code diff:

-const toTrimmedString = (v: unknown): string =>
-  typeof v === 'string' ? v.trim() : '';
-
 export function getAggregatedMetricCustomization(
   metricId: string,
   options: {
     config: Config;
   },
 ): MetricCustomization {
-  const configPath = `scorecard.plugins.${metricId}.homepage.aggregatedMetric`;
+  const toTrimmedString = (v: unknown): string =>
+    typeof v === 'string' ? v.trim() : '';
+

Encapsulate the toTrimmedString helper function by moving it inside
getAggregatedMetricCustomization to clarify its scope and avoid polluting the
module scope.

workspaces/scorecard/plugins/scorecard-backend/src/utils/getAggregatedMetricCustomization.ts [26-27]

-const toTrimmedString = (v: unknown): string =>
-  typeof v === 'string' ? v.trim() : '';
+export function getAggregatedMetricCustomization(
+  metricId: string,
+  options: {
+    config: Config;
+  },
+): MetricCustomization {
+  const toTrimmedString = (v: unknown): string =>
+    typeof v === 'string' ? v.trim() : '';
 
+  const configPath = `scorecard.plugins.${metricId}.homepage.aggregatedMetric`;
+...
+
Suggestion importance[1-10]: 3

__

Why: This is a valid suggestion that improves code encapsulation for a minor style and readability improvement, but it has a low impact on the overall code quality.

Low
Security
Validate metric ID path segments
Suggestion Impact:The commit adds metricId splitting/validation, builds configPath from provider and metricName, and updates InputError messages to reference the computed configPath (though it throws on invalid metricId instead of returning a default).

code diff:

-  const configPath = `scorecard.plugins.${metricId}.homepage.aggregatedMetric`;
+  const toTrimmedString = (v: unknown): string =>
+    typeof v === 'string' ? v.trim() : '';
+
+  const [provider, metricName, ...rest] = metricId.split('.');
+
+  if (!provider || !metricName || rest.length > 0) {
+    throw new InputError(
+      `Invalid metric ID: ${metricId}, must be in the format "provider.metricName"`,
+    );
+  }
+
+  const configPath = `scorecard.plugins.${provider}.${metricName}.homepage.aggregatedMetric`;
 

Validate the metricId format to ensure it consists of exactly two parts
(provider.metricName) before using it to construct a configuration path.

workspaces/scorecard/plugins/scorecard-backend/src/utils/getAggregatedMetricCustomization.ts [29-64]

 export function getAggregatedMetricCustomization(
   metricId: string,
   options: {
     config: Config;
   },
 ): MetricCustomization {
-  const configPath = `scorecard.plugins.${metricId}.homepage.aggregatedMetric`;
+  const [provider, metricName, ...rest] = metricId.split('.');
+  if (!provider || !metricName || rest.length > 0) {
+    return { title: '', description: '', isCustomized: false };
+  }
+
+  const configPath = `scorecard.plugins.${provider}.${metricName}.homepage.aggregatedMetric`;
 
   const title = options.config.getOptional(`${configPath}.title`);
   const description = options.config.getOptional(`${configPath}.description`);
 
   if (title === undefined && description === undefined) {
     return { title: '', description: '', isCustomized: false };
   }
 
   if (title === undefined || description === undefined) {
     throw new InputError(
-      `scorecard.plugins.${metricId}.homepage.aggregatedMetric requires both title and description when customizing aggregated KPI`,
+      `${configPath} requires both title and description when customizing aggregated KPI`,
     );
   }
 
   const trimmedTitle = toTrimmedString(title);
   const trimmedDescription = toTrimmedString(description);
 
   if (!trimmedTitle || !trimmedDescription) {
     throw new InputError(
-      `scorecard.plugins.${metricId}.homepage.aggregatedMetric requires both title and description to be non-empty strings`,
+      `${configPath} requires both title and description to be non-empty strings`,
     );
   }
 
   return {
     title: trimmedTitle,
     description: trimmedDescription,
     isCustomized: true,
   };
 }

[Suggestion processed]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that metricId comes from a URL parameter and is used to construct a config path, so it should be validated. This improves the robustness and security of the new function.

Medium
Possible issue
Only emit customization flag when needed

Conditionally include the customized field in the response metadata only when
its value is true, aligning the implementation with its optional (customized?:
boolean) definition in the API.

workspaces/scorecard/plugins/scorecard-backend/src/service/mappers.ts [47-64]

 static toAggregatedMetricResult(
   metric: Metric,
   aggregatedMetric: AggregatedMetric,
   customized: boolean = false,
 ): AggregatedMetricResult {
   return {
     id: metric.id,
     status: 'success',
     metadata: {
       title: metric.title,
       description: metric.description,
       type: metric.type,
       history: metric.history,
-      customized,
+      ...(customized ? { customized: true } : {}),
     },
     result: aggregatedMetric,
   };
 }

[Suggestion processed]

Suggestion importance[1-10]: 4

__

Why: The suggestion correctly points out that the implementation should align with the API contract where customized is an optional field. This is a good practice for API consistency, though its impact is minor.

Low
General
Only override metric fields
Suggestion Impact:The code was changed to destructure { isCustomized, title, description } from getAggregatedMetricCustomization and only apply title/description to the metric when customized, instead of spreading the whole customization object. It also propagates these fields (and isCustomized) in the metrics listing response.

code diff:

+    return res.json({
+      metrics: getMetrics().map(metric => {
+        const { isCustomized, title, description } =
+          getAggregatedMetricCustomization(metric.id, { config });
+
+        return isCustomized
+          ? { ...metric, isCustomized, title, description }
+          : metric;
+      }),
+    });
   });
 
   router.get('/metrics/catalog/:kind/:namespace/:name', async (req, res) => {
@@ -191,22 +200,19 @@
         metricId,
       );
 
-    const aggregatedMetricCustomization = getAggregatedMetricCustomization(
-      metricId,
-      { config },
-    );
-
-    if (aggregatedMetricCustomization.isCustomized) {
-      const result = AggregatedMetricMapper.toAggregatedMetricResult(
-        { ...metric, ...aggregatedMetricCustomization },
+    const { isCustomized, title, description } =
+      getAggregatedMetricCustomization(metricId, { config });
+
+    const metricsDetails = isCustomized
+      ? { ...metric, title, description }
+      : metric;
+
+    return res.json(
+      AggregatedMetricMapper.toAggregatedMetricResult(
+        metricsDetails,
         aggregatedMetric,
-        aggregatedMetricCustomization.isCustomized,
-      );
-      return res.json(result);
-    }
-
-    return res.json(
-      AggregatedMetricMapper.toAggregatedMetricResult(metric, aggregatedMetric),
+        isCustomized,
+      ),
     );

In router.ts, explicitly set the title and description properties on the metric
object instead of spreading the entire aggregatedMetricCustomization object to
avoid unintended side effects.

workspaces/scorecard/plugins/scorecard-backend/src/service/router.ts [199-206]

 if (aggregatedMetricCustomization.isCustomized) {
   const result = AggregatedMetricMapper.toAggregatedMetricResult(
-    { ...metric, ...aggregatedMetricCustomization },
+    {
+      ...metric,
+      title: aggregatedMetricCustomization.title,
+      description: aggregatedMetricCustomization.description,
+    },
     aggregatedMetric,
-    aggregatedMetricCustomization.isCustomized,
+    true,
   );
   return res.json(result);
 }

[Suggestion processed]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out that spreading the entire aggregatedMetricCustomization object is unsafe, as it could unintentionally override metric properties or add unwanted fields like isCustomized.

Medium
Possible issue
Fix optional config typing
Suggestion Impact:Changed homepage.aggregatedMetric to be optional (aggregatedMetric?:) in the type definition.

code diff:

           homepage?: {
-            aggregatedMetric: {
+            aggregatedMetric?: {

In config.d.ts, mark the aggregatedMetric property as optional within the
homepage type definition to align with its optional usage and improve
configuration flexibility.

workspaces/scorecard/plugins/scorecard-backend/config.d.ts [45-50]

 homepage?: {
-  aggregatedMetric: {
+  aggregatedMetric?: {
     title: string;
     description: string;
   };
 };

[Suggestion processed]

Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies that the aggregatedMetric property should be optional within the homepage object to improve type flexibility and prevent validation errors for valid partial configurations.

Low

Copy link
Member

@Eswaraiahsapram Eswaraiahsapram left a comment

Choose a reason for hiding this comment

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

Thanks @imykhno, for this PR.

I added a few comments PTAL.

Comment on lines 64 to 94
Copy link
Member

Choose a reason for hiding this comment

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

It might be cleaner to compute cardTitle and cardDescription first and have a single return block to avoid duplication.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, I updated the logic

Comment on lines 54 to 62
if (aggregatedScorecard.result?.total === 0) {
return (
<EmptyStatePanel
metricId={metricId}
label={t('errors.noDataFound')}
tooltipContent={t('errors.noDataFoundMessage')}
/>
);
}
Copy link
Member

Choose a reason for hiding this comment

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

Since we are introducing customization for aggregated KPI title and description, should we also consider updating the EmptyStatePanel title and description accordingly?

Image

API Response for Jira

{
  "id": "jira.open_issues",
  "status": "success",
  "metadata": {
    "title": "My Jira issues",
    "description": "Custom description for Jira issues",
    "type": "number",
    "history": true,
    "customized": true
  },
  "result": {
    "values": [
      {
        "count": 0,
        "name": "success"
      },
      {
        "count": 0,
        "name": "warning"
      },
      {
        "count": 0,
        "name": "error"
      }
    ],
    "total": 0,
    "timestamp": "2026-02-24T05:35:52.273Z"
  }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes you are totally right, this behavior didn't work. Fixed with the final changes

plugins:
jira:
open_issues:
homepage:
Copy link
Member

Choose a reason for hiding this comment

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

In theory, aggregated cards can be shown on any page, so maybe we don't need to bake in homepage.
But it is true that the component is named ScorecardHomepageCard 😁

Copy link
Member

Choose a reason for hiding this comment

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

I agree with you. In theory, aggregated cards can be used on any page, so tying it to the homepage doesn’t feel ideal.

During the initial implementation, we somehow ended up naming it ScorecardHomepageCard. When we get some time, I think we should definitely consider renaming it to something more generic and future-proof.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would say, we need to support current "homepage" approach for now to make a code more consistent. Moreover, as @Eswaraiahsapram mentioned, when we have some time for renaming, I think, it would be much easier to rename rather than one part follows "homepage" pattern and another does not

Copy link
Member

Choose a reason for hiding this comment

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

You could avoid the breaking change by for example using name: aggregations in app-config.

jira:
open_issues:
homepage:
aggregatedMetric:
Copy link
Member

Choose a reason for hiding this comment

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

Could you please think about how this configuration might change when users will be able to specify also other aggregation scorecards like sum that is part of the feature? Because that will influence how the config can look like. For example, now if new Scorecard for sum aggregation is added, that needs a different title, there is no way to specify it in config.
I would also rather use name aggregatedScorecards.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you please think about how this configuration might change when users will be able to specify also other aggregation scorecards like sum that is part of the feature?

I would say, that I expect to add some kind of type for the configuration. By default it will be "Sum", also user can provide the type as "Average" which affect the logic of aggregation and card on IU.

For example, now if new Scorecard for sum aggregation is added, that needs a different title, there is no way to specify it in config.

Sorry, I didn't understand what do you mean in this example. Currently supported only two cards and user can customize them, did you mean some third card?

I would also rather use name aggregatedScorecards.

As we already under the scorecard configuration I expect that all configuration inside will be affect the plugin, so I am not sure of tautology

I will be waiting for your response, thank you!

description: metric.description,
type: metric.type,
history: metric.history,
customized,
Copy link
Member

Choose a reason for hiding this comment

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

You use isCustomized and customized, can you please unify it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, updated to isCustomized for all places

Signed-off-by: Ihor Mykhno <imykhno@redhat.com>
@redhat-developer redhat-developer deleted a comment from rhdh-qodo-merge bot Feb 25, 2026
…stomization handling

Signed-off-by: Ihor Mykhno <imykhno@redhat.com>
Signed-off-by: Ihor Mykhno <imykhno@redhat.com>
…oduce route constants

Signed-off-by: Ihor Mykhno <imykhno@redhat.com>
…anel

Signed-off-by: Ihor Mykhno <imykhno@redhat.com>
Signed-off-by: Ihor Mykhno <imykhno@redhat.com>
Signed-off-by: Ihor Mykhno <imykhno@redhat.com>
@imykhno
Copy link
Contributor Author

imykhno commented Feb 27, 2026

In the latest update, I added logic to allow title and description customization for cards with failed statuses by refactoring the EmptyStatePanel.

We can no longer use hardcoded text for these cards because we need to determine if a card has been customized. Consequently, I updated the ErrorStatePanel to support an additional request for card details if an error occurs during aggregation. (This approach was previously discussed but had not yet been implemented). I utilized the GET /metrics endpoint for this, while it was already designed for this approach, I updated the logic to ensure it supports customization as well.

Since the GET /metrics endpoint was not previously in use, I have also added the necessary error messages and their respective translations.

Finally, I refactored workspaces/scorecard/plugins/scorecard/src/api/index.ts to unify method descriptions and validations. I believe that if a method returns a specific type, it is better to validate the response data in our case at the data getter layer rather than within the individual use... hooks.

Below, I have added screenshots showing how the page reacts to the customization:

1. Cards displays without customization. Translation supported

Without customization

2. Cards were customizatzed. The first card displays behavior with successful aggregation, the second card displays "No data found" error. Translation is not supported

With customization

3. Cards were customizatzed. Card displays error which happened during loading aggregated data. Translation is not supported

Error durring loading aggregated KPI

I will be appreciate for re-review!

CC @Eswaraiahsapram @dzemanov

…I customization

Signed-off-by: Ihor Mykhno <imykhno@redhat.com>
@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 3, 2026

Copy link
Member

@Eswaraiahsapram Eswaraiahsapram left a comment

Choose a reason for hiding this comment

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

Thanks @imykhno , I’ve added a few very minor comments to the PR. When you have a moment, could you please take another look?

import Box from '@mui/material/Box';
import CircularProgress from '@mui/material/CircularProgress';

export const CardLoading = () => {
Copy link
Member

Choose a reason for hiding this comment

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

We can move this to a Common folder so that it can be reused in other places. we can pass height as prop.

label={t('errors.metricDataUnavailable')}
tooltipContent={t('errors.userNotFoundInCatalogMessage')}
/>
const getPanelContent = () => {
Copy link
Member

Choose a reason for hiding this comment

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

Can we use useMemo and rename it to getErrorPanelContent?

@imykhno
Copy link
Contributor Author

imykhno commented Mar 6, 2026

The architecture for this task has changed. Therefore, I am closing this PR and will open a new one. The new PR will include the relevant code from this branch along with the new updates, and all previous comments will be taken into account.
Thanks to everyone for the review!

@imykhno imykhno closed this Mar 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants