fix: incorrect storage group count

[astandrik]

Closes #2650

Stand

CI Results

Test Status: ⚠️ FLAKY

📊 Full Report

Total	Passed	Failed	Flaky	Skipped
354	347	0	5	2

Test Changes Summary ⏭️2

⏭️ Skipped Tests (2)

1. Scroll to row, get shareable link, navigate to URL and verify row is scrolled into view (tenant/diagnostics/tabs/queries.test.ts)
2. Copy result button copies to clipboard (tenant/queryEditor/queryEditor.test.ts)

Bundle Size: ✅

Current: 85.36 MB | Main: 85.36 MB
Diff: +0.00 KB (0.00%)

✅ Bundle size unchanged.

ℹ️ CI Information

• Test recordings for failed tests are available in the full report.
• Bundle size is measured for the entire 'dist' directory.
• 📊 indicates links to detailed reports.
• 🔺 indicates increase, 🔽 decrease, and ✅ no change in bundle size.

[astandrik]

bugbot run
/claude_review

[claude]

Claude finished @astandrik's task —— View job

---

PR Review: Fix incorrect storage group count

I've reviewed the changes in this PR that addresses issue #2650. Here's my comprehensive feedback:

✅ Positive Aspects

1. Proper API Usage: The fix correctly uses the storage API with RTK Query to fetch the actual storage group count instead of relying on the array length
2. Capability Checking: Good use of capability hooks (useCapabilitiesLoaded, useStorageGroupsHandlerAvailable) to ensure the groups handler is available before making the API call
3. Polling Integration: Properly integrates with the existing auto-refresh mechanism using useAutoRefreshInterval
4. Backward Compatibility: The implementation gracefully falls back to displaying just "Storage" when the groups handler is not available

🔍 Issues and Suggestions

1. Missing PropTypes Documentation (src/containers/Tenant/Diagnostics/TenantOverview/MetricsTabs/MetricsTabs.tsx:33)

The tenantName prop was added to the interface but there's no JSDoc comment explaining its purpose. Consider adding documentation for consistency.

2. Simplified TabCard Interface (src/containers/Tenant/Diagnostics/TenantOverview/TabCard/TabCard.tsx)

The removal of the label prop and keeping only text is a good simplification. However, ensure all usages throughout the codebase have been updated.

3. Performance Consideration (src/containers/Tenant/Diagnostics/TenantOverview/MetricsTabs/MetricsTabs.tsx:60-72)

The storage groups API call is made with limit: 0 which is efficient for just getting the count. Good optimization choice.

4. Conditional Text Display (src/containers/Tenant/Diagnostics/TenantOverview/MetricsTabs/MetricsTabs.tsx:139-142)

The conditional rendering logic for storage text is clean and follows the i18n pattern correctly:

text={
    storageGroupCount === undefined
        ? i18n('cards.storage-label')
        : i18n('context_storage-groups', {count: storageGroupCount})
}

✅ Code Standards Compliance

• TypeScript: Type safety is maintained throughout the changes
• i18n: All user-facing strings use the i18n system correctly
• API Patterns: Follows the RTK Query pattern with proper hooks
• React Patterns: Uses hooks appropriately and maintains component purity
• BEM Naming: CSS classes follow the BEM convention with cn() utility

📋 Testing Recommendations

While the code changes look good, consider adding tests for:

1. The behavior when groupsHandlerAvailable is false
2. The API call parameters validation
3. The conditional text rendering based on storageGroupCount

Summary

The fix correctly addresses the issue of incorrect storage group count by fetching the actual count from the API instead of using the array length. The implementation follows all the project's coding standards and best practices. The changes are minimal and focused on solving the specific problem.

[artemmufazalov]

You have StorageGroups inside tenant data, there is no need to use storage handler

https://github.com/ydb-platform/ydb-embedded-ui/blob/main/src/types/api/tenant.ts#L56

[astandrik]

/claude_review

[Copilot]

Pull Request Overview

This PR fixes an incorrect storage group count display by passing the actual storage groups count from tenant data instead of calculating it from storage statistics array length. The fix ensures that the storage tab shows the correct number of storage groups as provided by the backend API.

Key Changes

• Pass storageGroupsCount from tenant data through the component hierarchy
• Simplify TabCard component by consolidating label and sublabel props into a single text prop
• Update storage group count logic to use actual tenant data instead of derived statistics

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
TenantOverview.tsx	Pass storageGroupsCount from tenantData.StorageGroups to MetricsTabs component
TabCard.tsx	Simplify props by replacing label and sublabel with single text prop
MetricsTabs.tsx	Accept storageGroupsCount prop and use it instead of calculating from stats array length

[Copilot]

Using Number() constructor without validation could cause issues if tenantData.StorageGroups contains non-numeric values. Consider using parseInt() with explicit base or adding validation to ensure the value is numeric before conversion.

Suggested change

	tenantData.StorageGroups
	tenantData.StorageGroups &&
	!isNaN(Number(tenantData.StorageGroups))

[claude]

Claude finished @astandrik's task —— View job

---

PR Review: Fix incorrect storage group count

I've reviewed the changes that address issue #2650. The fix correctly passes the actual storage group count from tenant data instead of calculating it from the storage statistics array.

✅ Positive Aspects

1. Simple and Effective Fix: The solution directly uses tenantData.StorageGroups from the API response instead of deriving counts from storage statistics
2. Proper Type Safety: Uses TypeScript's optional chaining and proper type conversion with Number()
3. Backward Compatibility: Falls back to generic "Storage" label when count is undefined
4. Component Simplification: Consolidated TabCard props by removing redundant label/sublabel pattern

📋 Code Standards Review

TypeScript & Type Safety ✅

• Proper use of optional types (storageGroupsCount?: number)
• Type conversion is explicit with Number()
• All props are properly typed

React Best Practices ✅

• Props are passed cleanly through component hierarchy
• No unnecessary re-renders introduced
• Component interfaces remain consistent

Redux/RTK Patterns ✅

• No changes to state management
• Existing API data is reused efficiently
• No state mutations

i18n Usage ✅

storageGroupsCount === undefined
    ? i18n('cards.storage-label')
    : i18n('context_storage-groups', {count: storageGroupsCount})

• Proper use of i18n with parameterized strings
• Fallback text uses i18n keys
• Translation key exists in en.json

API Usage ✅

• Leverages existing tenant API data (tenantData.StorageGroups)
• No new API calls needed
• Follows window.api pattern implicitly through existing code

🔍 Minor Observations

1. Line 182-184 in TenantOverview.tsx: The conditional conversion could be simplified:

   storageGroupsCount={tenantData.StorageGroups ? Number(tenantData.StorageGroups) : undefined}

   This is fine, but be aware that Number("") returns 0, not NaN. The current implementation handles this correctly.

2. TabCard Interface Simplification: Good decision to remove the label/sublabel complexity. The single text prop is clearer.

⚡ Performance & Security

• Performance: No performance concerns. The fix reuses existing data without additional API calls or computations.
• Security: No security issues. No sensitive data exposure or new attack vectors.

✅ BEM Naming & Code Style

• All CSS classes use the BEM convention with cn() utility
• Code formatting is consistent with project standards

Summary

This is a well-implemented fix that addresses the issue correctly while maintaining all project standards. The change is minimal, focused, and follows the codebase conventions perfectly. The solution leverages existing API data efficiently without introducing complexity.

Great work on keeping the fix simple and maintainable! 👍