feat: restrictions for users without IsViewerAllowed role

[Raubzeug]

closes #2674
Stand

CI Results

Test Status: ⚠️ FLAKY

📊 Full Report

Total	Passed	Failed	Flaky	Skipped
358	353	0	3	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.35 MB
Diff: +6.44 KB (0.01%)

⚠️ Bundle size increased. Please review.

ℹ️ 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.

[Raubzeug]

/claude_review

[claude]

Claude finished @Raubzeug's task —— View job

---

Code Review: feat: restrictions for users without IsViewerAllowed role

📋 Review Summary

I've completed my review of this PR that implements role-based access restrictions for users without the IsViewerAllowed role. The implementation creates a three-tier permission system: full access users, viewer users, and restricted users.

✅ What Works Well

1. Consistent Permission Model: The implementation correctly establishes a clear hierarchy:

   • Full access: IsMonitoringAllowed === true
   • Viewer access: IsViewerAllowed === true && IsMonitoringAllowed === false
   • Restricted access: Both flags are false

2. Proper Hook Usage: All components correctly use centralized authentication hooks (useIsViewerUser() and useIsUserAllowedToMakeChanges())

3. Backward Compatibility: The implementation maintains backward compatibility by defaulting isUserAllowedToMakeChanges to true when the flag is not present

4. Type Safety: The changes follow TypeScript patterns correctly

🚨 Critical Issues

1. Hardcoded Strings - i18n Violation

src/components/VDiskPopup/VDiskPopup.tsx:36-92 contains multiple hardcoded strings that violate the i18n requirement:

const vdiskData: InfoViewerItem[] = [{label: 'State', value: 'not available'}];
// Multiple instances of hardcoded labels like 'StoragePool', 'NodeId', 'PDiskId', etc.

According to CLAUDE.md:299: "NEVER hardcode user-facing strings - use i18n". All these labels should use the i18n system.

2. Performance Concerns

src/store/reducers/cluster/cluster.ts:145-147:

const {currentData} = clusterApi.useGetClusterBaseInfoQuery(clusterNameFromQuery ?? skipToken, {
    skip: !isViewerUser,
});

This completely skips the cluster base info query for non-viewer users, which could cause:

• Components expecting this data to render empty states
• Potential null reference errors
• Inconsistent loading states

⚠️ Minor Issues

1. Potential Re-render Issues: While the components use useMemo correctly, ensure that the columns dependencies are also memoized in parent components to prevent unnecessary re-renders.

2. User Experience: The PDisk information is now conditionally rendered based on isViewerUser in VDiskPopup.tsx:264, but there's no indication to restricted users why this information is missing.

📝 Recommendations

1. Fix i18n violations immediately: Create proper i18n entries for all hardcoded strings following the pattern in i18n-naming-ruleset.md:

   // Instead of: {label: 'State', value: 'not available'}
   // Use: {label: i18n('field_state'), value: i18n('value_not-available')}

2. Handle skipped queries gracefully: Consider providing fallback data or explicit loading states when queries are skipped:

   const {currentData, isSkipped} = clusterApi.useGetClusterBaseInfoQuery(...);
   if (isSkipped) {
     return <RestrictedAccessMessage />;
   }

3. Add inline documentation: Document the permission hierarchy in key files to help future maintainers understand the access control system.

4. Consider error boundaries: Add error boundaries around components that might break when data is unavailable due to role restrictions.

✓ Checklist Results

• Read CLAUDE.md for repo-specific guidelines
• Review all changed files for code standards
• Check TypeScript type safety and interfaces - ✅ Good
• Verify i18n usage (no hardcoded strings) - ❌ FAILED - Multiple violations found
• Ensure API calls use window.api - ✅ Good
• Review Redux patterns and state management - ⚠️ Minor concerns with skipped queries
• Check performance and security considerations - ⚠️ Minor performance concerns
• Provide comprehensive feedback

🎯 Action Required

The hardcoded strings in VDiskPopup.tsx must be replaced with proper i18n keys before this PR can be merged. This is a critical requirement per the project guidelines.

[artemmufazalov]

I'd better turn off specific functions (tracing, monitoring links) in place, not here. From this query we receive balancer data, that can be used to properly generate links, cluster title, control plane for backups, etc.

[Raubzeug]

Currently we don't want to get access to cluster info from database. So, I think we should use something else for backups. I'll fix it!