-
Notifications
You must be signed in to change notification settings - Fork 0
Feat: Adds a modular dashboard a etter history ux #69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Updated widget API calls to match backend documentation requirements - Added widget transformation helper to convert frontend format to backend format - Fixed widget structure: widgets now send id, type, position, settings, layoutId as per API spec - Removed frontend-only properties (title, visible, refreshInterval) from API calls - Added proper error logging for debugging API issues - All widgets now include required settings field - Fixed TypeScript compilation errors
…gger spec
- Updated widget transformation to include title, refreshInterval, visible as per backend API
- Removed layoutId field which is not expected by backend
- Fixed widget structure to match Swagger documentation: /dashboard/layouts/{id} PUT endpoint
- Ensured all widgets have required fields with proper defaults
- Made visible property optional in WidgetConfig interface
- Removed unsupported widget types: 'server-status', 'quick-actions', 'chart' - Updated WidgetType to match backend enum exactly: * stats, activity-feed, alerts, resource-usage, user-presence, system-health, ups-status - Removed corresponding widget components that are not supported by backend - Updated widget catalog to only show valid widget types - This fixes the 400 error when adding widgets with invalid types
- Replace custom ID generation (type-timestamp) with crypto.randomUUID() - Fix validation error: 'id doit être un UUID valide' - Generate valid UUIDs for new widgets: e.g., 'a1b2c3d4-e5f6-7890-1234-567890abcdef' - Update default layout widgets to use proper UUIDs - This fixes the 400 error when backend validates widget ID format
- Add local state updates after successful API calls in store methods - Fix issue where widgets appear in backend but not in frontend after add/update/delete - Ensure addWidget, updateWidget, removeWidget, updateWidgetPositions sync local state - Now widgets display immediately after operations without needing page refresh - Resolves empty dashboard issue where PUT succeeds but UI shows no widgets
Fix TypeScript errors in dashboard widget components where they were referencing props.widget instead of props.config for accessing widget settings. Affected widgets: - AlertsWidget: showFilter and allowAcknowledge settings - ResourceUsageWidget: showChart setting - UserPresenceWidget: showRecentlyOffline setting
|
Caution Review failedThe pull request is closed. WalkthroughThis update introduces a comprehensive modular dashboard system, replacing the previous dashboard implementation with a new architecture supporting customizable layouts, widgets, templates, and user preferences. It also significantly enhances user and permission management, adds advanced history/statistics features, and improves localization. Numerous new Vue components, API methods, and TypeScript types are added, while obsolete files, mock data, and comments are removed. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DashboardView
participant DashboardStore
participant DashboardAPI
participant WidgetComponent
User->>DashboardView: Visit /dashboard
DashboardView->>DashboardStore: loadLayouts()
DashboardStore->>DashboardAPI: getLayouts()
DashboardAPI-->>DashboardStore: layouts[]
DashboardStore-->>DashboardView: layouts, activeLayout
DashboardView->>DashboardStore: loadPreferences()
DashboardStore->>DashboardAPI: getPreferences()
DashboardAPI-->>DashboardStore: preferences
DashboardStore-->>DashboardView: preferences
loop For each widget in activeLayout
DashboardView->>WidgetComponent: Render widget
WidgetComponent->>DashboardStore: fetchWidgetData(widget)
DashboardStore->>DashboardAPI: getWidgetData(widgetType)
DashboardAPI-->>DashboardStore: widgetData
DashboardStore-->>WidgetComponent: widgetData
end
User->>DashboardView: Edit layout/widgets/preferences
DashboardView->>DashboardStore: updateLayout/addWidget/etc.
DashboardStore->>DashboardAPI: Persist changes
DashboardAPI-->>DashboardStore: Updated data
DashboardStore-->>DashboardView: Updated state
Possibly related PRs
Suggested labels
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 48
🔭 Outside diff range comments (2)
src/features/setup/components/SetupProgress.vue (1)
195-198: Guard against divide-by-zero inprogressPercentage.
totalSteps.valuemay be0while the component boots or if the backend returns an empty list, producingInfinityand breaking the progress bar width calculation.
Patch:-const progressPercentage = computed(() => { - if (!props.setupStatus) return 0; - return ((currentStepIndex.value + 1) / totalSteps.value) * 100; -}); +const progressPercentage = computed(() => { + if (!props.setupStatus || totalSteps.value === 0) return 0; + return ((currentStepIndex.value + 1) / totalSteps.value) * 100; +});src/features/users/api.ts (1)
83-97: Use proper typing instead ofanyThe
payloadparameter is typed asany, which bypasses TypeScript's type safety. Consider using a proper type likeUserUpdateDtoor a partial version of it.export const updateUser = async ( id: string, - payload: any, + payload: Partial<UserUpdateDto>, token: string, ): Promise<UserResponseDto> => {
🧹 Nitpick comments (37)
src/features/auth/components/AuthCard.vue (1)
15-15: Remove empty<script setup>blockThe component currently has an empty
<script setup>block, which adds unnecessary bytes and visual noise. Unless you plan to add logic here soon, removing it will keep the SFC minimal.-<script setup lang="ts"></script>src/features/setup/components/SetupProgress.vue (1)
205-211: SimplifyhandleStepClickfor readability & maintainability.The two branches perform the same action (
emit('go-to-step', …)).
A single compound condition removes duplication and makes future edits safer.-const handleStepClick = (stepKey: string, index: number) => { - if (index === currentStepIndex.value) { - emit('go-to-step', stepKey); - } else if (index < currentStepIndex.value && stepKey === 'welcome') { - emit('go-to-step', stepKey); - } -}; +const handleStepClick = (stepKey: string, index: number) => { + if ( + index === currentStepIndex.value || + (index < currentStepIndex.value && stepKey === 'welcome') + ) { + emit('go-to-step', stepKey); + } +};package.json (1)
57-57: Caret‐range for Tailwind re-introduces nondeterminismChanging
tailwindcssfrom an exact3.4.17to^3.4.17allows any3.xpatch/minor.
Pros: receive bug-fixes automatically.
Cons: reproducible builds can break unexpectedly in CI/CD.If deterministic builds are critical, consider pinning with a lockfile or returning to an exact version.
src/features/servers/views/ServerDetailsView.vue (1)
223-223: Consider preserving context about SSH credential handling.The removed comment provided useful context about SSH credential management. While not critical, such comments can help future developers understand the intended behavior.
src/layouts/components/UserDropdown.vue (1)
28-28: Silent error handling may hide logout issues.While the logout flow continues correctly even if the API call fails, suppressing errors makes it harder to debug logout-related issues. Consider adding logging while maintaining the current behavior.
- } catch {} + } catch (error) { + console.warn('Logout API call failed:', error); + // Continue with local logout regardless of API failure + }src/features/setup/router/guard.ts (1)
32-32: Silent error handling may hide setup navigation issues.The empty catch block suppresses errors during setup status checks, which could make it difficult to debug setup-related navigation problems. Consider adding logging while maintaining the current fallback behavior.
- } catch { + } catch (error) { + console.warn('Setup status check failed:', error); + // Continue with normal navigation as fallback next(); }src/features/dashboard/components/widgets/StatsWidget.vue (1)
86-169: Consider extracting repetitive SVG iconsThe inline SVG definitions are quite repetitive and make the template harder to maintain. Consider extracting these to a dedicated icon component or using an icon library.
Example approach:
<Icon :name="stat.icon" class="w-6 h-6" />This would centralize icon definitions and reduce template size.
src/features/users/components/BitmaskExplanation.vue (1)
62-62: Consider dynamically generating the example based on actual permission values.The hard-coded example
READ (1) + WRITE (2) + RESTART (8) = 11might become outdated if permission bit values change. Consider generating this example dynamically using the actual permission values fromPermissionUtils.-<code class="mt-1 block text-xs text-indigo-600 dark:text-indigo-400"> - READ (1) + WRITE (2) + RESTART (8) = 11 -</code> +<code class="mt-1 block text-xs text-indigo-600 dark:text-indigo-400"> + {{ generateExampleCalculation() }} +</code>Add this computed property or method:
const generateExampleCalculation = () => { const perms = allPermissions.slice(0, 3); // Take first 3 permissions as example const sum = perms.reduce((acc, p) => acc + p.bit, 0); const parts = perms.map(p => `${$t(p.label)} (${p.bit})`).join(' + '); return `${parts} = ${sum}`; };src/features/dashboard/components/widgets/ActivityFeedWidget.vue (1)
143-157: Add fallback for unmapped activity types.The icon mapping might not cover all possible activity types. The fallback at line 156 is good, but consider logging unknown types to help identify missing mappings.
const getActivityIcon = (type: string) => { const icons: Record<string, string> = { server_created: 'server', server_deleted: 'server', server_updated: 'server', user_created: 'user-plus', user_deleted: 'user-minus', user_login: 'log-in', ups_alert: 'alert-triangle', room_created: 'home', vm_created: 'box', vm_deleted: 'box', }; - return icons[type] || 'activity'; + const icon = icons[type]; + if (!icon && import.meta.env.DEV) { + console.warn(`Unknown activity type for icon mapping: ${type}`); + } + return icon || 'activity'; };src/features/history/components/HistoryEventDetail.vue (2)
161-161: Use nullish coalescing for more robust fallbacks.Replace the logical OR operator with nullish coalescing to handle edge cases where values might be 0 or empty strings.
For example, update lines like:
-server: metadata?.serverId || 'Unknown', +server: metadata?.serverId ?? 'Unknown',-server: - metadata?.serverId || props.event.oldValue?.serverId || 'Unknown', +server: + metadata?.serverId ?? props.event.oldValue?.serverId ?? 'Unknown',This ensures that falsy but valid values (like 0) are not replaced with 'Unknown'.
Also applies to: 171-172, 180-182, 194-194, 203-204, 212-213
163-163: Consider validating and formatting bitmask values.The bitmask values are displayed directly without validation or formatting. Consider adding validation and potentially showing the permissions represented by the bitmask.
-bitmask: props.event.newValue?.bitmask || 0, +bitmask: formatBitmask(props.event.newValue?.bitmask ?? 0),Add a helper function:
const formatBitmask = (bitmask: number): string => { if (!Number.isInteger(bitmask) || bitmask < 0) { return '0 (Invalid)'; } // Optionally decode the permissions const permissions = PermissionUtils.decodeBitmask(bitmask); if (permissions.length > 0) { return `${bitmask} (${permissions.map(p => $t(p.label)).join(', ')})`; } return String(bitmask); };Also applies to: 172-173, 196-196, 204-205
src/features/dashboard/composables/useWidget.ts (1)
16-16: Use ReturnType for better type safety.The interval type should match what
window.setIntervalreturns, which can vary by environment.-let refreshInterval: number | null = null; +let refreshInterval: ReturnType<typeof setInterval> | null = null;This ensures type compatibility across different JavaScript environments.
Also applies to: 45-45, 55-56
src/features/dashboard/components/widgets/AlertsWidget.vue (1)
169-180: Consider caching alerts data.The alerts are fetched on every mount and refresh interval. Consider implementing caching in the dashboard store to reduce API calls when multiple instances of this widget are rendered.
You could implement a caching mechanism in the dashboard store:
// In dashboard store const alertsCache = ref<{ data: AlertsResponse | null; timestamp: number | null }>({ data: null, timestamp: null }); const getAlerts = async (forceRefresh = false) => { const now = Date.now(); const cacheAge = now - (alertsCache.value.timestamp || 0); if (!forceRefresh && alertsCache.value.data && cacheAge < 30000) { // 30 second cache return alertsCache.value.data; } const response = await api.getAlerts(); alertsCache.value = { data: response, timestamp: now }; return response; };src/features/dashboard/components/widgets/UpsStatusWidget.vue (1)
275-283: Verify interval cleanup and consider error handling.The refresh interval setup looks correct, but consider adding error handling for the periodic refresh to prevent silent failures.
if (props.config.refreshInterval) { refreshInterval = window.setInterval(() => { - fetchUpsStatus(); + fetchUpsStatus().catch(err => { + console.error('Failed to refresh UPS status:', err); + }); }, props.config.refreshInterval); }src/features/history/views/ModernHistoryView.vue (1)
54-65: Enhance export functionality for large datasetsThe current implementation might fail for large datasets due to URI length limitations, and lacks error handling.
Consider using Blob API for better performance and add error handling:
const exportData = () => { - const dataStr = JSON.stringify(events.value, null, 2); - const dataUri = - 'data:application/json;charset=utf-8,' + encodeURIComponent(dataStr); - - const exportFileDefaultName = `history-export-${new Date().toISOString().split('T')[0]}.json`; - - const linkElement = document.createElement('a'); - linkElement.setAttribute('href', dataUri); - linkElement.setAttribute('download', exportFileDefaultName); - linkElement.click(); + try { + const dataStr = JSON.stringify(events.value, null, 2); + const blob = new Blob([dataStr], { type: 'application/json' }); + const url = URL.createObjectURL(blob); + + const exportFileDefaultName = `history-export-${new Date().toISOString().split('T')[0]}.json`; + + const linkElement = document.createElement('a'); + linkElement.href = url; + linkElement.download = exportFileDefaultName; + linkElement.click(); + + // Clean up + URL.revokeObjectURL(url); + } catch (error) { + console.error('Export failed:', error); + // Consider showing a user-friendly error message + } };src/features/dashboard/components/layout/DashboardGrid.vue (2)
111-138: Add explanatory comments to grid position calculationsThe grid position calculations are complex and would benefit from documentation for maintainability.
Add comments explaining the calculation logic:
const getGridPosition = (event: DragEvent): { x: number; y: number } | null => { if (!gridContainer.value) return null; const rect = gridContainer.value.getBoundingClientRect(); const padding = 16; + // Calculate mouse position relative to grid container (accounting for padding) const relativeX = event.clientX - rect.left - padding; const relativeY = event.clientY - rect.top - padding; if (relativeX < 0 || relativeY < 0) { return null; } const gap = 16; const totalWidth = rect.width - 2 * padding; + // Calculate cell width considering gaps between cells const cellWidth = (totalWidth - gap * (props.layout.columns - 1)) / props.layout.columns; const cellPlusGap = cellWidth + gap; + // Convert pixel coordinates to grid coordinates const gridX = Math.floor(relativeX / cellPlusGap); const gridY = Math.floor((relativeY + gap) / (props.layout.rowHeight + gap)); if (gridX < 0 || gridX >= props.layout.columns) { return null; } return { x: gridX, y: Math.max(0, gridY) }; };
179-183: Document the purpose of custom drag imageThe code creates a custom transparent drag image, but the purpose isn't immediately clear.
Add a comment explaining why this approach is used:
+ // Use a transparent 1x1 image to hide the default drag ghost image + // This allows us to show our custom preview instead const dragImage = new Image(); dragImage.src = 'data:image/svg+xml,%3Csvg width="1" height="1" xmlns="http://www.w3.org/2000/svg"%3E%3C/svg%3E'; event.dataTransfer.setDragImage(dragImage, 0, 0);src/features/history/components/HistoryStats.vue (1)
62-72: Consider colorblind-friendly palette for chartsThe current color palette may not be accessible to colorblind users. Consider using a colorblind-friendly palette or adding patterns.
Use a colorblind-friendly color palette:
backgroundColor: [ - '#3b82f6', - '#10b981', - '#f59e0b', - '#ef4444', - '#8b5cf6', - '#ec4899', - '#06b6d4', - '#14b8a6', - '#f97316', + '#1f77b4', // blue + '#ff7f0e', // orange + '#2ca02c', // green + '#d62728', // red + '#9467bd', // purple + '#8c564b', // brown + '#e377c2', // pink + '#7f7f7f', // gray + '#bcbd22', // olive ],Consider also adding the chartjs-plugin-datalabels plugin to display values directly on the chart segments.
src/features/dashboard/components/widgets/UserPresenceWidget.vue (1)
170-171: Consider using proper typing for browser timerThe type
number | nullmight not be compatible withwindow.setIntervalreturn type in all TypeScript configurations. Consider using a more specific type.-let refreshInterval: number | null = null; +let refreshInterval: ReturnType<typeof setInterval> | null = null;src/features/users/components/RolePermissions.vue (1)
420-430: Extract error message handling logicThe error message extraction logic is duplicated. Consider creating a utility function.
// Add this utility function const extractErrorMessage = (error: any, defaultMessage: string): string => { return error?.response?.data?.message?.[0] || error?.response?.data?.message || defaultMessage; }; // Then use it in both places: const errorMessage = extractErrorMessage(error, t('permissions.updateError'));Also applies to: 465-471
src/features/history/components/HistoryEventTable.vue (1)
331-344: Consider extracting entity icons to constantsThe entity icons mapping could be moved to a constant or imported from a shared constants file for reusability.
// Consider moving to a shared constants file export const ENTITY_ICONS: Record<string, string> = { auth: '🔐', user: '👤', server: '🖥️', vm: '💻', room: '🏢', ups: '🔋', role: '👑', group: '👥', user_role: '🎭', permission_server: '🔑', permission_vm: '🗝️', };src/features/groups/views/GroupView.vue (1)
179-183: Consider adding minimal error handling to the empty catch block.While the cleanup effort is appreciated, completely empty catch blocks can silently hide important errors. Consider adding at least basic error tracking or logging.
try { await groupStore.fetchAllGroups(); - } catch (error) {} + } catch (error) { + // TODO: Add appropriate error handling + }src/features/history/types.ts (1)
78-78: Remove redundant interface declaration.The
HistoryFiltersinterface is redundant as it extendsHistoryFilterwithout adding any properties.-export interface HistoryFilters extends HistoryFilter {}src/features/users/components/UserResetPasswordModal.vue (2)
36-41: Consider strengthening password validation.The current minimum length of 6 characters may be insufficient for security. Consider implementing more comprehensive password requirements.
const isFormValid = computed(() => { return ( - newPassword.value.length >= 6 && - confirmPassword.value.length >= 6 && + newPassword.value.length >= 8 && + confirmPassword.value.length >= 8 && + /^(?=.*[a-z])(?=.*[A-Z])(?=.*\d)(?=.*[!@#$%^&*])/.test(newPassword.value) && passwordsMatch.value ); });
84-86: Consider making success delay configurable.The hardcoded 2-second delay might not be optimal for all use cases. Consider making it configurable or reducing it.
setTimeout(() => { handleClose(); - }, 2000); + }, 1500);src/features/users/components/PermissionCreator.vue (2)
334-358: Improve error handling specificity.The current error handling is generic. Consider providing more specific error messages based on the type of failure.
} catch (err) { - error.value = t('permissions.loadResourcesError'); + if (err.response?.status === 404) { + error.value = t('permissions.noResourcesFound'); + } else if (err.response?.status === 403) { + error.value = t('permissions.accessDenied'); + } else { + error.value = t('permissions.loadResourcesError'); + } } finally {
384-404: Add validation before API calls.Consider adding additional validation before making API calls to prevent unnecessary network requests.
async function createPermission() { if (!canCreate.value) return; + if (!selectedResourceId.value || selectedPermissions.value.length === 0) { + error.value = t('permissions.invalidSelection'); + return; + } isSaving.value = true; error.value = null;src/features/users/components/PermissionEditor.vue (2)
215-215: Consider reactivity concerns with object copying.The shallow copy of the permission object might not maintain reactivity properly if the prop changes. Consider using a watcher to sync changes.
const localPermission = ref({ ...props.permission }); +watch( + () => props.permission, + (newPermission) => { + localPermission.value = { ...newPermission }; + }, + { deep: true } +);
235-242: Optimize select all permissions implementation.The current implementation calls
PermissionUtils.addfor each permission individually. This could be optimized to calculate the full bitmask at once.function selectAllPermissions() { - allPermissions.forEach((perm) => { - localPermission.value.bitmask = PermissionUtils.add( - localPermission.value.bitmask, - perm, - ); - }); + localPermission.value.bitmask = allPermissions.reduce((acc, perm) => acc | perm, 0); }src/features/dashboard/views/ModularDashboardView.vue (2)
387-465: Consider validating widget size constraintsThe widget definitions don't validate that
minSizevalues are smaller than or equal tomaxSize. This could lead to invalid configurations.Consider adding a validation function:
const validateWidgetDefinition = (def: WidgetDefinition): boolean => { return def.minSize.w <= def.maxSize.w && def.minSize.h <= def.maxSize.h; }; // Use it when processing definitions const validDefinitions = widgetDefinitions.filter(validateWidgetDefinition);
127-188: Enhance modal accessibilityThe widget catalog modal could benefit from improved accessibility features.
<div v-if="showWidgetCatalog" class="fixed inset-0 bg-black bg-opacity-50 flex items-center justify-center z-50" @click.self="showWidgetCatalog = false" + role="dialog" + aria-modal="true" + aria-labelledby="widget-catalog-title" > <div class="bg-white dark:bg-neutral-800 rounded-xl shadow-2xl p-4 sm:p-6 max-w-4xl w-full max-h-[80vh] overflow-y-auto m-4" > - <h2 class="text-2xl font-bold mb-4 dark:text-white"> + <h2 id="widget-catalog-title" class="text-2xl font-bold mb-4 dark:text-white"> {{ t('dashboard.widget_catalog') }} </h2>src/features/users/components/UserEditRoleModal.vue (1)
90-101: Simplify role toggle logicThe role toggle function can be simplified for better readability.
const toggleRole = (roleId: string) => { if (!form.value.roleIds) { form.value.roleIds = [roleId]; return; } const index = form.value.roleIds.indexOf(roleId); if (index > -1) { form.value.roleIds.splice(index, 1); } else { form.value.roleIds.push(roleId); } };src/features/users/views/UserListView.vue (1)
237-240: Consistent event namingThe component listens to both
@editUserand@editRoleevents that call the same handler. Consider using a single, more descriptive event name.- @editUser="handleEditUser" - @editRole="handleEditUser" + @editUserRole="handleEditUser"dashboard-api-documentation.md (1)
492-492: Expand widget settings documentationThe documentation mentions widget settings but doesn't provide details about what settings are available for each widget type.
Consider adding a section documenting the available settings for each widget type. For example:
### Widget-Specific Settings #### Activity Feed Widget - `limit`: Number of activities to display (default: 10) - `types`: Array of activity types to filter (optional) #### Alerts Widget - `severity`: Array of severity levels to display (default: ["critical", "warning", "info"]) - `autoAcknowledge`: Boolean to auto-acknowledge after timeout (default: false) #### Resource Usage Widget - `period`: Time period for historical data ("1h", "24h", "7d", "30d") - `metrics`: Array of metrics to display (["cpu", "memory", "storage", "network"])src/features/dashboard/types.ts (1)
48-49: Consider making refreshInterval required with a defaultThe
refreshIntervalis optional but the API always provides a default value of 30000ms. Making it required would improve type safety.- refreshInterval?: number; + refreshInterval: number; // default: 30000src/features/dashboard/store.ts (2)
267-267: Document or make configurable the magic numberThe addition of 5 extra rows is a magic number that should be documented or made configurable.
+ // Add 5 extra rows as buffer for new widget placement const grid = Array(maxRows + widgetHeight + 5)Or make it configurable:
const GRID_BUFFER_ROWS = 5; const grid = Array(maxRows + widgetHeight + GRID_BUFFER_ROWS)
411-430: Extract download utility for reusabilityThe file download logic using DOM manipulation could be extracted into a reusable utility function.
Create a utility function:
// utils/download.ts export function downloadBlob(blob: Blob, filename: string) { const url = URL.createObjectURL(blob); const a = document.createElement('a'); a.href = url; a.download = filename; document.body.appendChild(a); a.click(); document.body.removeChild(a); URL.revokeObjectURL(url); }Then simplify the store method:
- const url = URL.createObjectURL(blob); - const a = document.createElement('a'); - a.href = url; - a.download = `widget-${widgetId}-export.${format}`; - document.body.appendChild(a); - a.click(); - document.body.removeChild(a); - URL.revokeObjectURL(url); + downloadBlob(blob, `widget-${widgetId}-export.${format}`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (107)
AGENTS.md(0 hunks)dashboard-api-documentation.md(1 hunks)package.json(1 hunks)src/components/CommandPalette.vue(0 hunks)src/components/CommandPalette/useCommandPalette.ts(0 hunks)src/composables/useApiCache.ts(0 hunks)src/composables/useToast.ts(1 hunks)src/features/auth/components/AuthCard.vue(1 hunks)src/features/auth/components/RegisterForm.vue(0 hunks)src/features/auth/store.ts(0 hunks)src/features/auth/views/LoginView.vue(1 hunks)src/features/auth/views/RegisterView.vue(0 hunks)src/features/changelog/__tests__/api.spec.ts(0 hunks)src/features/changelog/components/TeamSection.vue(0 hunks)src/features/changelog/mock.ts(0 hunks)src/features/changelog/views/ChangelogView.vue(0 hunks)src/features/dashboard/__tests__/api.spec.ts(0 hunks)src/features/dashboard/api.ts(3 hunks)src/features/dashboard/components/DashboardCharts.vue(0 hunks)src/features/dashboard/components/layout/DashboardGrid.vue(1 hunks)src/features/dashboard/components/layout/LayoutManager.vue(1 hunks)src/features/dashboard/components/layout/TemplatesModal.vue(1 hunks)src/features/dashboard/components/widgets/ActivityFeedWidget.vue(1 hunks)src/features/dashboard/components/widgets/AlertsWidget.vue(1 hunks)src/features/dashboard/components/widgets/BaseWidget.vue(1 hunks)src/features/dashboard/components/widgets/ResourceUsageWidget.vue(1 hunks)src/features/dashboard/components/widgets/StatsWidget.vue(1 hunks)src/features/dashboard/components/widgets/SystemHealthWidget.vue(1 hunks)src/features/dashboard/components/widgets/UpsStatusWidget.vue(1 hunks)src/features/dashboard/components/widgets/UserPresenceWidget.vue(1 hunks)src/features/dashboard/components/widgets/index.ts(1 hunks)src/features/dashboard/composables/useWidget.ts(1 hunks)src/features/dashboard/store.ts(2 hunks)src/features/dashboard/types.ts(1 hunks)src/features/dashboard/types/widget.ts(1 hunks)src/features/dashboard/views/DashboardView.vue(0 hunks)src/features/dashboard/views/ModularDashboardView.vue(1 hunks)src/features/groups/components/CreateGroupModal.vue(0 hunks)src/features/groups/components/GroupDetailsModal.vue(0 hunks)src/features/groups/components/GroupEditModal.vue(1 hunks)src/features/groups/components/GroupManagementPanel.vue(0 hunks)src/features/groups/components/PriorityInput.vue(4 hunks)src/features/groups/components/ResourceSelector.vue(0 hunks)src/features/groups/components/ShutdownPreviewDialog.vue(1 hunks)src/features/groups/components/SimpleResourceSelector.vue(0 hunks)src/features/groups/components/panel/EditModeContent.vue(0 hunks)src/features/groups/components/panel/ViewModeContent.vue(0 hunks)src/features/groups/stores/usePriorityStore.ts(6 hunks)src/features/groups/types.ts(0 hunks)src/features/groups/utils/priority-utils.ts(0 hunks)src/features/groups/views/GroupView.vue(2 hunks)src/features/history/api.ts(3 hunks)src/features/history/components/AdvancedHistoryFilters.vue(1 hunks)src/features/history/components/HistoryEventCard.vue(1 hunks)src/features/history/components/HistoryEventDetail.vue(3 hunks)src/features/history/components/HistoryEventTable.vue(1 hunks)src/features/history/components/HistoryFilters.vue(0 hunks)src/features/history/components/HistoryStats.vue(1 hunks)src/features/history/components/InfiniteScrollHistory.vue(1 hunks)src/features/history/store.ts(4 hunks)src/features/history/types.ts(4 hunks)src/features/history/views/HistoryListView.vue(0 hunks)src/features/history/views/ModernHistoryView.vue(1 hunks)src/features/presence/composables/usePresenceSocket.ts(2 hunks)src/features/roles/components/AssignUsersModal.vue(0 hunks)src/features/roles/mock.ts(0 hunks)src/features/roles/store.test.ts(0 hunks)src/features/roles/store.ts(2 hunks)src/features/roles/views/RolesAdminView.vue(3 hunks)src/features/rooms/views/RoomDetails.vue(0 hunks)src/features/rooms/views/RoomListView.vue(0 hunks)src/features/servers/components/CreateServer.vue(0 hunks)src/features/servers/components/ServerEditModal.vue(3 hunks)src/features/servers/store.ts(1 hunks)src/features/servers/views/ServerDetailsView.vue(1 hunks)src/features/setup/components/SetupProgress.vue(1 hunks)src/features/setup/components/steps/CompleteStep.vue(0 hunks)src/features/setup/components/steps/CreateRoomStep.vue(0 hunks)src/features/setup/components/steps/CreateServerStep.vue(0 hunks)src/features/setup/components/steps/CreateUpsStep.vue(0 hunks)src/features/setup/components/steps/VmDiscoveryStep.vue(0 hunks)src/features/setup/router/guard.ts(1 hunks)src/features/setup/store.ts(0 hunks)src/features/setup/views/SetupView.vue(0 hunks)src/features/ups/api.ts(0 hunks)src/features/users/api.ts(3 hunks)src/features/users/components/BitmaskExplanation.vue(1 hunks)src/features/users/components/PermissionCreator.vue(1 hunks)src/features/users/components/PermissionEditor.vue(1 hunks)src/features/users/components/RolePermissions.vue(1 hunks)src/features/users/components/UserCreateModal.vue(1 hunks)src/features/users/components/UserEditRoleModal.vue(1 hunks)src/features/users/components/UserResetPasswordModal.vue(1 hunks)src/features/users/composables/useUsers.ts(1 hunks)src/features/users/mock.ts(0 hunks)src/features/users/permission-api.ts(1 hunks)src/features/users/types.ts(1 hunks)src/features/users/views/UserListView.vue(7 hunks)src/features/vms/api.ts(1 hunks)src/features/vms/types.ts(1 hunks)src/layouts/components/TreeNavbar.vue(0 hunks)src/layouts/components/UserDropdown.vue(1 hunks)src/layouts/views/DefaultLayout.vue(1 hunks)src/locales/en.json(13 hunks)src/locales/fr.json(13 hunks)src/main.ts(0 hunks)src/router/index.ts(3 hunks)
⛔ Files not processed due to max files limit (10)
- src/services/api.ts
- src/shared/components/ConfirmModal.vue
- src/shared/components/Icon.vue
- src/shared/components/Modal.vue
- src/shared/components/UserRolesDisplay.vue
- src/shared/composables/useCommandPalette.ts
- src/shared/composables/useToaster.ts
- src/shared/utils/permissions.ts
- src/styles.d.ts
- src/vite-env.d.ts
💤 Files with no reviewable changes (42)
- src/features/auth/views/RegisterView.vue
- src/components/CommandPalette/useCommandPalette.ts
- src/features/dashboard/tests/api.spec.ts
- src/features/roles/components/AssignUsersModal.vue
- src/features/roles/store.test.ts
- src/main.ts
- src/features/groups/types.ts
- src/features/changelog/components/TeamSection.vue
- src/features/ups/api.ts
- src/features/changelog/tests/api.spec.ts
- src/features/groups/components/ResourceSelector.vue
- src/features/servers/components/CreateServer.vue
- src/composables/useApiCache.ts
- src/components/CommandPalette.vue
- src/features/rooms/views/RoomDetails.vue
- src/features/setup/views/SetupView.vue
- src/features/groups/components/panel/EditModeContent.vue
- src/features/groups/utils/priority-utils.ts
- src/features/rooms/views/RoomListView.vue
- src/features/setup/components/steps/CreateUpsStep.vue
- src/features/setup/store.ts
- src/features/setup/components/steps/CreateServerStep.vue
- src/layouts/components/TreeNavbar.vue
- src/features/groups/components/panel/ViewModeContent.vue
- src/features/auth/components/RegisterForm.vue
- src/features/setup/components/steps/CompleteStep.vue
- src/features/auth/store.ts
- src/features/setup/components/steps/CreateRoomStep.vue
- src/features/changelog/mock.ts
- src/features/users/mock.ts
- src/features/groups/components/GroupManagementPanel.vue
- AGENTS.md
- src/features/groups/components/CreateGroupModal.vue
- src/features/changelog/views/ChangelogView.vue
- src/features/groups/components/SimpleResourceSelector.vue
- src/features/groups/components/GroupDetailsModal.vue
- src/features/setup/components/steps/VmDiscoveryStep.vue
- src/features/dashboard/views/DashboardView.vue
- src/features/history/views/HistoryListView.vue
- src/features/roles/mock.ts
- src/features/history/components/HistoryFilters.vue
- src/features/dashboard/components/DashboardCharts.vue
🧰 Additional context used
🧬 Code Graph Analysis (13)
src/features/dashboard/components/widgets/index.ts (2)
src/features/dashboard/types.ts (1)
WidgetType(34-41)src/features/dashboard/types/widget.ts (1)
WidgetType(9-16)
src/features/users/api.ts (1)
src/features/users/types.ts (2)
UserCreateDto(27-34)UserResponseDto(3-14)
src/features/roles/store.ts (1)
src/features/roles/types.ts (1)
RoleWithUsers(64-67)
src/features/dashboard/composables/useWidget.ts (2)
src/features/dashboard/types.ts (1)
Widget(43-51)src/features/dashboard/types/widget.ts (1)
WidgetData(56-61)
src/features/vms/api.ts (1)
src/features/vms/types.ts (5)
CreateVmPayload(25-29)Vm(1-9)UpdateVmPayload(31-34)VmListParams(18-23)VmListResponse(11-16)
src/features/history/store.ts (2)
src/features/history/types.ts (1)
HistoryStatsResponse(119-125)src/features/history/api.ts (1)
historyApi(39-69)
src/features/dashboard/types/widget.ts (1)
src/features/dashboard/types.ts (3)
WidgetType(34-41)WidgetPosition(27-32)DashboardLayout(53-63)
src/features/history/api.ts (1)
src/features/history/types.ts (1)
HistoryStatsResponse(119-125)
src/features/users/types.ts (1)
src/features/roles/types.ts (2)
PermissionServerDto(1-5)PermissionVmDto(7-11)
src/features/users/permission-api.ts (1)
src/features/users/types.ts (2)
PermissionServerDto(43-48)PermissionVmDto(54-59)
src/features/dashboard/store.ts (5)
src/features/dashboard/types.ts (10)
DashboardLayout(53-63)DashboardPreferences(65-73)DashboardTemplate(75-81)Widget(43-51)ActivityFeedResponse(100-107)AlertsResponse(123-130)ResourceUsageResponse(140-149)UserPresenceResponse(160-172)SystemHealthResponse(182-187)UpsStatusResponse(201-210)src/features/dashboard/types/widget.ts (1)
DashboardLayout(35-42)src/composables/useToast.ts (1)
useToast(6-46)src/composables/useApiCache.ts (1)
useApiCache(6-59)src/features/dashboard/api.ts (1)
dashboardApi(62-269)
src/features/dashboard/api.ts (2)
src/features/dashboard/types.ts (10)
Widget(43-51)DashboardLayout(53-63)ActivityFeedResponse(100-107)AlertsResponse(123-130)ResourceUsageResponse(140-149)UserPresenceResponse(160-172)SystemHealthResponse(182-187)UpsStatusResponse(201-210)DashboardPreferences(65-73)DashboardTemplate(75-81)src/features/dashboard/types/widget.ts (1)
DashboardLayout(35-42)
src/features/dashboard/types.ts (1)
src/features/dashboard/types/widget.ts (3)
WidgetPosition(18-23)WidgetType(9-16)DashboardLayout(35-42)
🔇 Additional comments (81)
src/features/groups/components/PriorityInput.vue (3)
61-62: LGTM! Good formatting improvement.Splitting the long CSS class string across multiple lines improves readability while maintaining the same functionality.
88-89: LGTM! Consistent formatting improvement.Breaking up the long color class strings across multiple lines maintains consistency with the formatting style and improves readability.
Also applies to: 96-97
108-113: LGTM! Better readability for the watch function.Reformatting the watch function from inline to multi-line format improves code readability and follows Vue 3 composition API best practices.
src/features/servers/store.ts (1)
114-114: LGTM: Formatting improvement enhances code consistency.The formatting adjustment to the
findIndexcallback improves code readability and consistency without affecting functionality.src/features/servers/components/ServerEditModal.vue (3)
41-62: LGTM: Improved readability with multi-line watch function format.The reformatting of the watch function from a single-line to multi-line format significantly improves code readability and maintainability while preserving the exact same functionality.
78-78: LGTM: Clean whitespace removal.The removal of trailing whitespace improves code cleanliness.
212-214: LGTM: Better template formatting for readability.The multi-line formatting of the h3 tag improves template readability and consistency with the overall codebase style.
package.json (3)
52-52:globshould be a build-time-only dependency
globis declared underdevDependencies, which is correct if it’s only used by tooling (e.g. Vite plug-ins or code-gen).
Double-check that it is not required at runtime; otherwise it must move todependencies.
54-54: PostCSS minor bump → re-run Tailwind & autoprefixer testsUpgrading from
^8.5.3→^8.5.6is a patch-level bump, still within v8, but watch for:
• outdated PostCSS plug-ins pinning<8.5.6
• changed peer-dependency ranges in Tailwind/Autoprefixer.
Runpnpm build && pnpm testlocally to ensure CSS pipeline is unaffected.
16-39: No GSAP usage detected – safe to remove
A broad search for “gsap” (imports, requires, dynamic imports, and plain mentions) outside of package.json returned no references. Removing the dependency will not break runtime code.src/features/servers/views/ServerDetailsView.vue (1)
145-148: LGTM: Improved code formatting.The multi-line formatting of the function call improves readability.
src/features/vms/types.ts (1)
1-34: Well-structured TypeScript interfaces for VM management.The type definitions are comprehensive and follow TypeScript best practices. The interfaces effectively support CRUD operations, pagination, and filtering.
Minor suggestion for consistency: Consider adding JSDoc comments for interface documentation:
+/** + * Represents a virtual machine entity + */ export interface Vm { id: string; name: string; // ... rest of properties }This would improve developer experience when using these types.
src/layouts/views/DefaultLayout.vue (1)
57-57: Minor arrow function syntax improvement.Adding parentheses around the arrow function parameter improves code consistency and readability.
src/router/index.ts (2)
135-136: ModernHistoryView.vue VerifiedThe
ModernHistoryView.vuecomponent exists and correctly uses<script setup>and a<template>section, so the updated routing is valid.
22-23: Dashboard routing update verifiedThe new
ModularDashboardView.vuefile exists with a proper<script setup>block and<template>section. The routing changes on lines 22–23 (and the mirrored import on lines 38–39) align with the PR’s modular dashboard objective—approved for merge.src/features/users/composables/useUsers.ts (1)
42-46: Improved error handling by removing mock data fallback.The removal of mock data fallback in error scenarios is a good improvement that eliminates dependency on mock data and provides more realistic error handling.
Consider enhancing the user experience by providing a more informative error state:
} catch { users.value = []; totalItems.value = 0; totalPages.value = 0; currentPage.value = 1; isMock.value = false; + // Consider emitting an error event or setting a specific error message + // to help users understand what went wrong }This ensures users understand when and why the user list is empty.
src/features/dashboard/components/widgets/index.ts (1)
12-20: Verify widget component mapping consistency.The widget component mapping looks well-structured, but ensure all widget types are covered and match the type definitions exactly.
The centralized widget registry approach is excellent for the modular dashboard architecture. The mapping provides good type safety and enables dynamic widget rendering.
src/features/roles/store.ts (4)
52-53: Approve the removal of mock data fallbacks.Replacing mock data fallbacks with empty arrays provides a cleaner error state and prevents confusion between real and mock data.
80-81: Consistent error handling improvement.The approach of returning empty user arrays on individual role fetch failures is appropriate and maintains data integrity.
89-90: Clean error state management.Setting empty arrays and
isMock: falseprovides a clear, consistent error state across the store.
539-539: Simplified error handling is appropriate.Removing the console.error logging in the catch block is a good simplification, especially since error handling is likely managed at higher levels.
src/features/history/api.ts (3)
2-6: Import addition looks good.The addition of
HistoryStatsResponseto the imports is appropriate for the new stats API method.
22-31: Excellent consistency improvement in filter handling.The enhanced
sanitizeFiltersfunction now handles both singular (entity,action) and plural (entities,actions) forms consistently. This improves the API's flexibility and user experience.
63-68: Well-implemented stats API method.The new
getStatsmethod follows the established patterns in the codebase with proper authentication headers and type safety.src/features/roles/views/RolesAdminView.vue (3)
445-445: Clean component import and integration.The import of
RolePermissionsfrom the users feature is appropriate and maintains proper feature separation.
368-380: Excellent integration of role permissions display.The new permissions card with the
RolePermissionscomponent is well-integrated:
- Proper binding to the selected role
- Appropriate event handling for updates
- Consistent styling with the rest of the component
539-539: Appropriate error handling simplification.The simplified catch block aligns with the pattern seen in the store file and is appropriate for this context.
src/composables/useToast.ts (3)
1-4: Well-defined toast options interface.The
ToastOptionsinterface is clean and covers the essential toast notification properties with appropriate defaults.
17-37: Excellent type-specific convenience functions.The convenience functions (
showSuccess,showError,showWarning,showInfo) provide a clean API while maintaining type safety by omitting thetypeproperty from options.
39-46: Clean composable return interface.The return object exposes all necessary functions for comprehensive toast functionality.
src/features/presence/composables/usePresenceSocket.ts (1)
68-85: Good implementation of interval lifecycle management!The ping interval is properly managed with cleanup in all scenarios - clearing existing intervals before creating new ones, and cleaning up on both disconnect and component unmount. The conditional registration of
onBeforeUnmountonly when a Vue component instance exists is a thoughtful approach.src/features/dashboard/components/widgets/BaseWidget.vue (3)
36-118: LGTM! Well-structured widget header with proper accessibility.The header implementation is excellent with:
- Proper button accessibility using
titleattributes- Clean SVG icons with consistent styling
- Responsive design considerations
- Clear semantic structure
122-150: LGTM! Proper state management for loading and error states.The conditional rendering logic is well-implemented:
- Loading spinner with consistent styling
- Error state with clear visual feedback
- Proper slot usage for content rendering
154-203: LGTM! Comprehensive responsive styling with smooth transitions.The CSS implementation demonstrates:
- Smooth hover effects with proper transforms
- Responsive breakpoints for mobile and tablet
- Consistent dark mode support
- Clean use of Tailwind utilities
src/features/history/store.ts (4)
26-28: LGTM! Well-defined reactive state for pagination and statistics.The new state variables are properly typed and follow Vue 3 composition API best practices:
hasMorefor pagination controlstatsfor statistics data with proper nullable typestatsLoadingfor loading state management
30-47: LGTM! Enhanced fetchHistory method with proper pagination support.The implementation correctly:
- Adds optional
appendparameter for infinite scroll- Properly manages array concatenation vs replacement
- Updates pagination state correctly
- Maintains existing error handling
63-72: LGTM! Clean statistics fetching implementation.The
fetchStatsmethod follows the established pattern:
- Proper loading state management
- Correct error handling with try/finally
- Consistent API integration
- Returns data for potential chaining
82-90: LGTM! Complete store interface with new functionality.The return object properly exposes all new state and methods, maintaining consistency with the existing store pattern.
src/features/vms/api.ts (3)
2-8: LGTM! Comprehensive type imports for enhanced type safety.The import statement properly includes all necessary types for VM operations, ensuring type safety across the API module.
14-21: LGTM! Improved function signatures with proper TypeScript typing.The enhanced functions demonstrate:
createUvms: Proper payload typing and generic response typepatchVm: Type-safe parameters and direct data return- Consistent error handling patterns
23-41: LGTM! Well-implemented new API functions with consistent patterns.The new functions follow excellent practices:
fetchVmById: Simple, type-safe single VM retrievalfetchVms: Comprehensive paginated listing with default parametersgetVmsAdmin: Admin-specific endpoint with proper typing- Consistent async/await patterns and data extraction
src/features/history/components/HistoryEventCard.vue (8)
18-24: LGTM! Clean time formatting with proper internationalization.The
formatTimefunction correctly handles locale-specific time formatting with appropriate fallbacks.
26-36: LGTM! Robust user display logic with proper fallbacks.The
getUserDisplayfunction handles various user data scenarios gracefully:
- Prioritizes full name over email
- Handles missing user data
- Provides meaningful fallbacks
38-50: LGTM! Comprehensive entity icon mapping.The icon mapping covers all major entity types with appropriate emojis, providing good visual distinction.
52-66: LGTM! Well-structured action badge color mapping.The color coding system effectively categorizes actions:
- Green for creation operations
- Blue for updates
- Red for deletions and failures
- Purple for start/restart operations
- Orange for shutdown operations
68-74: LGTM! Safe entity linking logic.The computed property properly validates entity path existence and entity ID validity before allowing link creation.
76-96: LGTM! Comprehensive quick info extraction.The
getQuickInfofunction handles various entity types and provides meaningful contextual information for different event types.
99-210: LGTM! Rich and accessible event display template.The template implementation is excellent:
- Clear visual hierarchy with timeline dot and content
- Proper responsive design
- Conditional rendering based on data availability
- Accessible button interactions
- Good use of icons and visual feedback
212-226: LGTM! Clean and responsive component styling.The scoped styles provide:
- Consistent card appearance with hover effects
- Proper timeline visual elements
- Good contrast for action badges
- Responsive design considerations
src/features/dashboard/components/widgets/UpsStatusWidget.vue (5)
1-20: LGTM! Clean integration with BaseWidget and proper error handling.The template properly uses the BaseWidget wrapper and implements standard loading/error states with localized messaging.
22-61: LGTM! Well-organized summary statistics display.The grid layout effectively displays key UPS metrics:
- Total count with proper typography
- Status-specific counts with color coding
- Responsive design with consistent spacing
63-81: LGTM! Effective average load visualization.The progress bar implementation provides clear visual feedback for average load with:
- Proper percentage display
- Dynamic color coding based on load level
- Smooth transitions
83-176: LGTM! Comprehensive individual UPS display.The detailed UPS listing is well-implemented:
- Status-based styling with proper color coding
- Battery and load progress bars
- Additional metrics (runtime, temperature, test dates)
- Proper scrollable container for multiple devices
234-273: LGTM! Comprehensive utility functions for visual styling.The utility functions provide:
- Consistent color mapping for different states
- Proper threshold-based color coding
- Good separation of concerns
src/features/dashboard/components/widgets/UserPresenceWidget.vue (1)
172-184: Well-implemented error handlingThe async data fetching with proper error handling and loading state management is well done. The try-catch-finally pattern ensures the loading state is always updated.
src/features/history/components/HistoryEventTable.vue (1)
7-42: Excellent accessibility implementationGreat job on the table accessibility:
- Proper use of
scopeattributes- Screen reader only text for actions column
- Semantic HTML structure
src/features/history/components/AdvancedHistoryFilters.vue (2)
52-97: Excellent UX with date presetsThe date preset functionality provides great user experience by offering quick selections for common date ranges. The implementation is clean and maintainable.
309-313: Well-implemented indeterminate checkbox stateThe logic for handling indeterminate checkbox states for group selections is correctly implemented. This provides clear visual feedback about partial selections.
src/features/dashboard/components/layout/TemplatesModal.vue (2)
120-131: LGTM! Solid error handling and state management.The
loadTemplatesfunction properly manages loading and error states with try-catch-finally pattern, ensuring the UI stays responsive even when API calls fail.
133-138: Good defensive programming in createLayout function.The early return guard clause ensures the function only proceeds when a template is selected, preventing potential runtime errors.
src/features/users/types.ts (1)
27-34: Well-defined user creation interface.The
UserCreateDtointerface properly defines all required fields for user creation with appropriate types and optional role assignment.src/features/groups/views/GroupView.vue (1)
277-279: Good cleanup of stub functions.The simplified no-op implementations are appropriate for stub functions and maintain the expected async interface.
src/features/dashboard/components/layout/LayoutManager.vue (3)
321-327: Excellent lifecycle management for event listeners.The component properly adds and removes the click outside event listener in the mounted/unmounted hooks, preventing memory leaks.
311-319: Robust click outside detection logic.The
handleClickOutsidefunction correctly usesclosest()to check for menu elements and prevents closing when clicking within menu boundaries.
250-259: Good user experience pattern for new layout creation.Creating a new layout and immediately setting it as active provides intuitive feedback to users about which layout they're now working with.
src/features/users/components/UserCreateModal.vue (4)
67-75: Secure password generation implementation.The password generation function creates strong passwords with a good mix of characters and appropriate length (12 characters). The character set includes special characters which enhances security.
38-47: Comprehensive form validation logic.The computed
isFormValidproperty correctly validates all required fields including minimum password length and role selection, providing good user feedback.
77-97: Proper error handling and API integration.The
handleSubmitfunction includes appropriate loading states, error handling with user-friendly messages, and proper cleanup after successful operations.
83-89: No guard needed forauthStore.token!here
The non-null assertion forauthStore.token!is used consistently across all API calls in this codebase, and the global route guard already ensures a valid token exists before this component is mounted. Adding a local null check here would be redundant.Likely an incorrect or invalid review comment.
src/features/history/types.ts (3)
11-12: LGTM: Well-named permission entity additions.The new
PermissionServerandPermissionVmentities follow consistent naming conventions and integrate well with the existing enum structure.
108-125: LGTM: Well-structured statistics interfaces.The new statistics interfaces provide comprehensive data structures for analytics features with clear, typed properties.
90-91: Confirm permission entity routing mappingThe
entityToPathentries forpermission_serverandpermission_vmboth resolve to/users/{id}, which drives the links in HistoryEventCard.vue:• src/features/history/types.ts (entityToPath)
• src/features/history/components/HistoryEventCard.vue (RouterLink:to="\/${entityToPath[event.entity]}/${event.entityId}`"`)Please verify that permission events should indeed navigate to user pages. If they should instead lead to server or VM permission views, update these mappings (e.g. to
'servers'or'vms') accordingly.src/features/users/components/UserResetPasswordModal.vue (1)
157-192: LGTM: Well-implemented password input fields.The password input fields include proper accessibility features, validation feedback, and user-friendly visibility toggles.
src/features/users/components/PermissionCreator.vue (2)
322-324: LGTM: Efficient bitmask calculation.The reduce operation for calculating the bitmask is clean and efficient.
406-407: LGTM: Proper success handling.The success flow with toast notification and modal closure provides good user feedback.
src/features/users/components/PermissionEditor.vue (3)
2-10: LGTM: Proper modal implementation.The Teleport usage and backdrop click handling follow Vue 3 best practices for modal components.
123-144: LGTM: Excellent permission visualization.The current bitmask display and active permissions badges provide clear feedback to users about their selections.
220-222: LGTM: Efficient computed property.The active permissions computed property efficiently calculates permissions from the bitmask using the utility function.
src/locales/fr.json (1)
59-228: High-quality French translationsThe new French translations for dashboard, user management, and permissions features are grammatically correct and maintain consistent terminology throughout. Good job on the localization effort!
Also applies to: 803-1721
src/features/users/components/UserEditRoleModal.vue (1)
108-347: Well-implemented modal UIThe modal implementation has good accessibility features with proper labels, keyboard navigation, and loading states. The tab navigation provides a clean UX for separating concerns.
dashboard-api-documentation.md (1)
245-245: Remove redundantlastUpdatedfieldThe stats response includes a
lastUpdatedfield that isn't defined in theFullDashboardStatsDtointerface in the codebase.- }, - "lastUpdated": "2025-01-05T12:00:00Z" + }Likely an incorrect or invalid review comment.
| } catch (error) { | ||
| console.error('Error checking setup:', error); | ||
| } | ||
| } catch (error) {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Silent error handling may hide important setup issues.
The empty catch block suppresses all errors from setupApi.getPublicStatus(), which could make it difficult to debug setup-related problems. Consider adding proper error handling or logging for better visibility.
- } catch (error) {}
+ } catch (error) {
+ console.warn('Failed to check setup status:', error);
+ // Fallback to default behavior when setup status is unavailable
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (error) {} | |
| } catch (error) { | |
| console.warn('Failed to check setup status:', error); | |
| // Fallback to default behavior when setup status is unavailable | |
| } |
🤖 Prompt for AI Agents
In src/features/auth/views/LoginView.vue at line 50, the catch block for errors
from setupApi.getPublicStatus() is empty, which silently suppresses any errors
and hinders debugging. Modify the catch block to include proper error handling
by logging the error or handling it appropriately to ensure visibility of any
issues during setup.
| } catch (error) { | ||
| console.error('Failed to execute shutdown:', error); | ||
| } | ||
| } catch (error) {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Silent error handling in shutdown operation.
The empty catch block suppresses errors during group shutdown execution, which is dangerous because:
- Users won't know if the shutdown failed
- The UI still emits 'success' and 'close' events on failure
- This could lead to users believing the shutdown was successful when it actually failed
- } catch (error) {}
+ } catch (error) {
+ console.error('Shutdown execution failed:', error);
+ groupStore.error = error.message || 'Shutdown execution failed';
+ // Don't emit success/close events on failure
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (error) {} | |
| } catch (error) { | |
| console.error('Shutdown execution failed:', error); | |
| groupStore.error = error.message || 'Shutdown execution failed'; | |
| // Don't emit success/close events on failure | |
| } |
🤖 Prompt for AI Agents
In src/features/groups/components/ShutdownPreviewDialog.vue at line 288, the
catch block is empty, causing silent error handling during the shutdown
operation. Modify the catch block to properly handle errors by logging or
displaying an error message to the user, and prevent emitting 'success' and
'close' events when a failure occurs. This ensures users are informed of
failures and the UI reflects the actual shutdown status.
| state: server.state, | ||
| })); | ||
| } catch (err) { | ||
| } catch { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider preserving error context for debugging purposes.
The removal of error parameters from catch blocks eliminates valuable debugging information. While this reduces console noise, it makes troubleshooting API failures more difficult.
Consider preserving the original error details while avoiding console logging:
- } catch {
+ } catch (error) {
error.value = 'Failed to fetch servers';
servers.value = [];
+ // Preserve error details for debugging without console logging
+ if (import.meta.env.DEV) {
+ console.error('Priority store error:', error);
+ }
}Apply similar changes to other catch blocks to maintain error context during development.
Also applies to: 63-63, 81-81, 98-98, 123-123, 148-148
🤖 Prompt for AI Agents
In src/features/groups/stores/usePriorityStore.ts at lines 43, 63, 81, 98, 123,
and 148, the catch blocks currently omit the error parameter, losing valuable
error context. Modify each catch block to include the error parameter (e.g.,
catch (error)) and preserve the error object for debugging purposes without
necessarily logging it to the console. This will maintain error context for
troubleshooting API failures while avoiding unwanted console noise.
| servers.value = originalServers; | ||
| error.value = 'Failed to update server priority'; | ||
| throw err; | ||
| throw new Error('Failed to update server priority'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Generic Error objects lose original error information.
Throwing new generic Error objects instead of rethrowing the original errors removes valuable debugging context such as HTTP status codes, response details, and network error information.
Consider preserving the original error while providing user-friendly messages:
- } catch {
+ } catch (originalError) {
servers.value = originalServers;
error.value = 'Failed to update server priority';
- throw new Error('Failed to update server priority');
+ throw new Error(`Failed to update server priority: ${originalError.message}`);
}This maintains the original error context while providing consistent error messages.
Also applies to: 101-101, 126-126, 151-151
🤖 Prompt for AI Agents
In src/features/groups/stores/usePriorityStore.ts at lines 84, 101, 126, and
151, the code throws new generic Error objects which discard the original error
details. To fix this, catch the original error and rethrow it or wrap it in a
new error while preserving the original error's message and stack trace. This
can be done by either throwing the original error directly or creating a new
error that includes the original error as a cause or appends its message,
ensuring debugging information like HTTP status and network errors are retained.
| @@ -0,0 +1,20 @@ | |||
| import type { Component } from 'vue'; | |||
| import type { WidgetType } from '../../types'; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Flag duplicate type definitions for consolidation.
The WidgetType is defined in both src/features/dashboard/types.ts and src/features/dashboard/types/widget.ts. This duplication could lead to inconsistencies if one is updated without the other.
Consider consolidating these type definitions into a single location to maintain consistency. Import the type from the primary location in both files that need it.
🏁 Script executed:
#!/bin/bash
# Description: Verify the duplicate WidgetType definitions are identical
# Expected: Both definitions should be identical
echo "=== WidgetType in types.ts ==="
rg -A 10 "export type WidgetType" src/features/dashboard/types.ts
echo "=== WidgetType in types/widget.ts ==="
rg -A 10 "export type WidgetType" src/features/dashboard/types/widget.tsLength of output: 685
Consolidate duplicate WidgetType definitions
Verified that WidgetType in both files is identical, but maintaining two copies risks future drift. Let’s centralize the definition and update imports accordingly:
• Remove export type WidgetType from one location (e.g. src/features/dashboard/types.ts)
• Keep it in a single file (e.g. src/features/dashboard/types/widget.ts)
• Update all imports to point at that file
Locations to update:
- src/features/dashboard/types.ts (remove definition)
- src/features/dashboard/components/widgets/index.ts (and any other consumers)
Example diff in src/features/dashboard/components/widgets/index.ts:
- import type { WidgetType } from '../../types';
+ import type { WidgetType } from '../../types/widget';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import type { WidgetType } from '../../types'; | |
| import type { WidgetType } from '../../types/widget'; |
🤖 Prompt for AI Agents
In src/features/dashboard/components/widgets/index.ts at line 2 and related
files, the WidgetType type is defined in multiple places causing duplication.
Remove the WidgetType definition from src/features/dashboard/types.ts and keep
it only in src/features/dashboard/types/widget.ts. Then update all imports of
WidgetType in src/features/dashboard/components/widgets/index.ts and any other
files to import from src/features/dashboard/types/widget.ts to ensure a single
source of truth.
| const generateRandomPassword = () => { | ||
| const chars = | ||
| 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789!@#$%^&*'; | ||
| let password = ''; | ||
| for (let i = 0; i < 12; i++) { | ||
| password += chars.charAt(Math.floor(Math.random() * chars.length)); | ||
| } | ||
| newPassword.value = password; | ||
| confirmPassword.value = password; | ||
| showPassword.value = true; | ||
| showConfirmPassword.value = true; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Improve password generation security.
The current random password generation uses Math.random() which is not cryptographically secure. For security-sensitive operations like password generation, use crypto.getRandomValues().
const generateRandomPassword = () => {
const chars =
'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789!@#$%^&*';
- let password = '';
- for (let i = 0; i < 12; i++) {
- password += chars.charAt(Math.floor(Math.random() * chars.length));
- }
+ const array = new Uint8Array(12);
+ crypto.getRandomValues(array);
+ let password = '';
+ for (let i = 0; i < 12; i++) {
+ password += chars.charAt(array[i] % chars.length);
+ }
newPassword.value = password;
confirmPassword.value = password;
showPassword.value = true;
showConfirmPassword.value = true;
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const generateRandomPassword = () => { | |
| const chars = | |
| 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789!@#$%^&*'; | |
| let password = ''; | |
| for (let i = 0; i < 12; i++) { | |
| password += chars.charAt(Math.floor(Math.random() * chars.length)); | |
| } | |
| newPassword.value = password; | |
| confirmPassword.value = password; | |
| showPassword.value = true; | |
| showConfirmPassword.value = true; | |
| }; | |
| const generateRandomPassword = () => { | |
| const chars = | |
| 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789!@#$%^&*'; | |
| const array = new Uint8Array(12); | |
| crypto.getRandomValues(array); | |
| let password = ''; | |
| for (let i = 0; i < 12; i++) { | |
| password += chars.charAt(array[i] % chars.length); | |
| } | |
| newPassword.value = password; | |
| confirmPassword.value = password; | |
| showPassword.value = true; | |
| showConfirmPassword.value = true; | |
| }; |
🤖 Prompt for AI Agents
In src/features/users/components/UserResetPasswordModal.vue around lines 57 to
68, replace the use of Math.random() for password generation with the Web Crypto
API's crypto.getRandomValues() to ensure cryptographically secure randomness.
Generate random indices securely using crypto.getRandomValues() to select
characters from the chars string, then build the password string as before. This
change will improve the security of the generated passwords.
| const servers = await getServersAdmin(); | ||
| availableResources.value = servers | ||
| .filter((s: any) => !props.existingServerIds.includes(s.id)) | ||
| .map((s: any) => ({ | ||
| id: s.id, | ||
| name: s.name, | ||
| description: s.host || s.ip, | ||
| })); | ||
| } else { | ||
| const vms = await getVmsAdmin(); | ||
| availableResources.value = vms | ||
| .filter((v) => !props.existingVmIds.includes(v.id)) | ||
| .map((v) => ({ | ||
| id: v.id, | ||
| name: v.name, | ||
| description: `Server: ${v.serverId}`, | ||
| })); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add type safety for API responses.
The server and VM API responses are typed as any, which reduces type safety. Consider adding proper TypeScript interfaces.
if (newType === 'server') {
const servers = await getServersAdmin();
availableResources.value = servers
- .filter((s: any) => !props.existingServerIds.includes(s.id))
- .map((s: any) => ({
+ .filter((s) => !props.existingServerIds.includes(s.id))
+ .map((s) => ({
id: s.id,
name: s.name,
description: s.host || s.ip,
}));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const servers = await getServersAdmin(); | |
| availableResources.value = servers | |
| .filter((s: any) => !props.existingServerIds.includes(s.id)) | |
| .map((s: any) => ({ | |
| id: s.id, | |
| name: s.name, | |
| description: s.host || s.ip, | |
| })); | |
| } else { | |
| const vms = await getVmsAdmin(); | |
| availableResources.value = vms | |
| .filter((v) => !props.existingVmIds.includes(v.id)) | |
| .map((v) => ({ | |
| id: v.id, | |
| name: v.name, | |
| description: `Server: ${v.serverId}`, | |
| })); | |
| } | |
| const servers = await getServersAdmin(); | |
| availableResources.value = servers | |
| .filter((s) => !props.existingServerIds.includes(s.id)) | |
| .map((s) => ({ | |
| id: s.id, | |
| name: s.name, | |
| description: s.host || s.ip, | |
| })); | |
| } else { | |
| const vms = await getVmsAdmin(); | |
| availableResources.value = vms | |
| .filter((v) => !props.existingVmIds.includes(v.id)) | |
| .map((v) => ({ | |
| id: v.id, | |
| name: v.name, | |
| description: `Server: ${v.serverId}`, | |
| })); | |
| } |
🤖 Prompt for AI Agents
In src/features/users/components/PermissionCreator.vue around lines 336 to 353,
the API responses for servers and VMs are currently typed as any, which reduces
type safety. Define TypeScript interfaces representing the expected structure of
server and VM objects, then update the getServersAdmin and getVmsAdmin calls to
use these interfaces instead of any. This will improve type safety and help
catch errors during development.
| "upsStatus": { | ||
| "error": "Échec du chargement de l'état des onduleurs", | ||
| "total": "Total", | ||
| "online": "En ligne", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix placeholder in email translation
The translation contains {email} which appears to be a placeholder that wasn't replaced with an actual example email.
- "email_placeholder": "utilisateur{'@'}exemple.com",
+ "email_placeholder": "[email protected]",Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/locales/fr.json at line 208, the translation string contains the
placeholder {email} which should be replaced with an actual example email or a
proper placeholder format. Update the translation to either remove the
placeholder or replace it with a valid example email address or a correctly
formatted placeholder that matches the application's interpolation syntax.
| const addWidget = async (type: WidgetType) => { | ||
| if (!activeLayout.value) return; | ||
| const definition = widgetDefinitions.find((d) => d.type === type); | ||
| if (!definition) return; | ||
| const newWidget: Widget = { | ||
| id: crypto.randomUUID(), | ||
| type, | ||
| position: { | ||
| x: 0, | ||
| y: 0, | ||
| w: definition.minSize.w, | ||
| h: definition.minSize.h, | ||
| }, | ||
| settings: {}, | ||
| title: t(`dashboard.widgets.${type}.name`), | ||
| refreshInterval: definition.refreshable ? 30000 : undefined, | ||
| visible: true, | ||
| }; | ||
| await dashboardStore.addWidget(activeLayout.value.id, newWidget); | ||
| showWidgetCatalog.value = false; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New widgets may overlap existing ones
New widgets are always positioned at {x: 0, y: 0} which will cause overlapping if that position is already occupied.
Consider implementing a function to find the next available position:
const findAvailablePosition = (widgets: Widget[], size: { w: number; h: number }): { x: number; y: number } => {
// Simple algorithm: find the lowest available row
const maxY = Math.max(0, ...widgets.map(w => w.position.y + w.position.h));
return { x: 0, y: maxY };
};
// Use it in addWidget
const position = findAvailablePosition(activeLayout.value.widgets, {
w: definition.minSize.w,
h: definition.minSize.h
});🤖 Prompt for AI Agents
In src/features/dashboard/views/ModularDashboardView.vue around lines 531 to
554, new widgets are always positioned at {x: 0, y: 0}, causing overlap with
existing widgets. To fix this, implement a function that calculates the next
available position by checking existing widgets' positions and sizes, then use
this function to assign a non-overlapping position to new widgets instead of the
fixed {x: 0, y: 0}.
| export const permissionServerApi = { | ||
| async getByRole( | ||
| roleId: string, | ||
| token: string, | ||
| ): Promise<PermissionServerDto[]> { | ||
| const { data } = await axios.get<PermissionServerDto[]>( | ||
| `/permissions/server/role/${roleId}`, | ||
| { | ||
| headers: { Authorization: `Bearer ${token}` }, | ||
| }, | ||
| ); | ||
| return data; | ||
| }, | ||
|
|
||
| async getByIds( | ||
| serverId: string, | ||
| roleId: string, | ||
| token: string, | ||
| ): Promise<PermissionServerDto> { | ||
| const { data } = await axios.get<PermissionServerDto>( | ||
| `/permissions/server/${serverId}/role/${roleId}`, | ||
| { | ||
| headers: { Authorization: `Bearer ${token}` }, | ||
| }, | ||
| ); | ||
| return data; | ||
| }, | ||
|
|
||
| async create( | ||
| dto: PermissionServerDto, | ||
| token: string, | ||
| ): Promise<PermissionServerDto> { | ||
| const { data } = await axios.post<PermissionServerDto>( | ||
| '/permissions/server', | ||
| dto, | ||
| { | ||
| headers: { Authorization: `Bearer ${token}` }, | ||
| }, | ||
| ); | ||
| return data; | ||
| }, | ||
|
|
||
| async update( | ||
| serverId: string, | ||
| roleId: string, | ||
| dto: { bitmask: number }, | ||
| token: string, | ||
| ): Promise<PermissionServerDto> { | ||
| const { data } = await axios.patch<PermissionServerDto>( | ||
| `/permissions/server/${serverId}/role/${roleId}`, | ||
| dto, | ||
| { | ||
| headers: { Authorization: `Bearer ${token}` }, | ||
| }, | ||
| ); | ||
| return data; | ||
| }, | ||
|
|
||
| async delete(serverId: string, roleId: string, token: string): Promise<void> { | ||
| await axios.delete(`/permissions/server/${serverId}/role/${roleId}`, { | ||
| headers: { Authorization: `Bearer ${token}` }, | ||
| }); | ||
| }, | ||
|
|
||
| async getUserPermissions( | ||
| userId: string, | ||
| token: string, | ||
| ): Promise<PermissionServerDto[]> { | ||
| const { data } = await axios.get<PermissionServerDto[]>( | ||
| `/permissions/server/user/${userId}`, | ||
| { | ||
| headers: { Authorization: `Bearer ${token}` }, | ||
| }, | ||
| ); | ||
| return data; | ||
| }, | ||
|
|
||
| async getMyPermissions(token: string): Promise<PermissionServerDto[]> { | ||
| const { data } = await axios.get<PermissionServerDto[]>( | ||
| '/permissions/server/user/me', | ||
| { | ||
| headers: { Authorization: `Bearer ${token}` }, | ||
| }, | ||
| ); | ||
| return data; | ||
| }, | ||
| }; | ||
|
|
||
| export const permissionVmApi = { | ||
| async getByRole(roleId: string, token: string): Promise<PermissionVmDto[]> { | ||
| const { data } = await axios.get<PermissionVmDto[]>( | ||
| `/permissions/vm/role/${roleId}`, | ||
| { | ||
| headers: { Authorization: `Bearer ${token}` }, | ||
| }, | ||
| ); | ||
| return data; | ||
| }, | ||
|
|
||
| async getByIds( | ||
| vmId: string, | ||
| roleId: string, | ||
| token: string, | ||
| ): Promise<PermissionVmDto> { | ||
| const { data } = await axios.get<PermissionVmDto>( | ||
| `/permissions/vm/${vmId}/role/${roleId}`, | ||
| { | ||
| headers: { Authorization: `Bearer ${token}` }, | ||
| }, | ||
| ); | ||
| return data; | ||
| }, | ||
|
|
||
| async create(dto: PermissionVmDto, token: string): Promise<PermissionVmDto> { | ||
| const { data } = await axios.post<PermissionVmDto>('/permissions/vm', dto, { | ||
| headers: { Authorization: `Bearer ${token}` }, | ||
| }); | ||
| return data; | ||
| }, | ||
|
|
||
| async update( | ||
| vmId: string, | ||
| roleId: string, | ||
| dto: { bitmask: number }, | ||
| token: string, | ||
| ): Promise<PermissionVmDto> { | ||
| const { data } = await axios.patch<PermissionVmDto>( | ||
| `/permissions/vm/${vmId}/role/${roleId}`, | ||
| dto, | ||
| { | ||
| headers: { Authorization: `Bearer ${token}` }, | ||
| }, | ||
| ); | ||
| return data; | ||
| }, | ||
|
|
||
| async delete(vmId: string, roleId: string, token: string): Promise<void> { | ||
| await axios.delete(`/permissions/vm/${vmId}/role/${roleId}`, { | ||
| headers: { Authorization: `Bearer ${token}` }, | ||
| }); | ||
| }, | ||
|
|
||
| async getUserPermissions( | ||
| userId: string, | ||
| token: string, | ||
| ): Promise<PermissionVmDto[]> { | ||
| const { data } = await axios.get<PermissionVmDto[]>( | ||
| `/permissions/vm/user/${userId}`, | ||
| { | ||
| headers: { Authorization: `Bearer ${token}` }, | ||
| }, | ||
| ); | ||
| return data; | ||
| }, | ||
|
|
||
| async getMyPermissions(token: string): Promise<PermissionVmDto[]> { | ||
| const { data } = await axios.get<PermissionVmDto[]>( | ||
| '/permissions/vm/user/me', | ||
| { | ||
| headers: { Authorization: `Bearer ${token}` }, | ||
| }, | ||
| ); | ||
| return data; | ||
| }, | ||
|
|
||
| async createBatch( | ||
| dto: BatchCreatePermissionDto, | ||
| token: string, | ||
| ): Promise<BatchPermissionResponse> { | ||
| const { data } = await axios.post<BatchPermissionResponse>( | ||
| '/permissions/vm/batch', | ||
| dto, | ||
| { | ||
| headers: { Authorization: `Bearer ${token}` }, | ||
| }, | ||
| ); | ||
| return data; | ||
| }, | ||
|
|
||
| async checkPermission( | ||
| dto: CheckPermissionDto, | ||
| token: string, | ||
| ): Promise<PermissionCheckResponse> { | ||
| const { data } = await axios.post<PermissionCheckResponse>( | ||
| '/permissions/vm/check', | ||
| dto, | ||
| { | ||
| headers: { Authorization: `Bearer ${token}` }, | ||
| }, | ||
| ); | ||
| return data; | ||
| }, | ||
| }; | ||
|
|
||
| export const permissionBatchApi = { | ||
| async createServerBatch( | ||
| dto: BatchCreatePermissionDto, | ||
| token: string, | ||
| ): Promise<BatchPermissionResponse> { | ||
| const { data } = await axios.post<BatchPermissionResponse>( | ||
| '/permissions/server/batch', | ||
| dto, | ||
| { | ||
| headers: { Authorization: `Bearer ${token}` }, | ||
| }, | ||
| ); | ||
| return data; | ||
| }, | ||
|
|
||
| async checkServerPermission( | ||
| dto: CheckPermissionDto, | ||
| token: string, | ||
| ): Promise<PermissionCheckResponse> { | ||
| const { data } = await axios.post<PermissionCheckResponse>( | ||
| '/permissions/server/check', | ||
| dto, | ||
| { | ||
| headers: { Authorization: `Bearer ${token}` }, | ||
| }, | ||
| ); | ||
| return data; | ||
| }, | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider centralizing authorization headers
The authorization header pattern is repeated in every API method. This could be centralized for better maintainability.
// Create a helper function
const getAuthHeaders = (token: string) => ({
headers: { Authorization: `Bearer ${token}` }
});
// Use it in API methods
async getByRole(roleId: string, token: string): Promise<PermissionServerDto[]> {
const { data } = await axios.get<PermissionServerDto[]>(
`/permissions/server/role/${roleId}`,
getAuthHeaders(token)
);
return data;
}Alternatively, consider configuring axios interceptors to automatically add the authorization header based on the stored token.
🤖 Prompt for AI Agents
In src/features/users/permission-api.ts from lines 37 to 259, the Authorization
header with the bearer token is repeated in every API method. To improve
maintainability, create a helper function that takes a token and returns the
headers object with the Authorization header set. Replace all inline header
objects in axios calls with calls to this helper function. Alternatively,
consider setting up an axios interceptor to automatically add the Authorization
header to all requests using the stored token.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review continued from previous batch...
| async createBatch( | ||
| dto: BatchCreatePermissionDto, | ||
| token: string, | ||
| ): Promise<BatchPermissionResponse> { | ||
| const { data } = await axios.post<BatchPermissionResponse>( | ||
| '/permissions/vm/batch', | ||
| dto, | ||
| { | ||
| headers: { Authorization: `Bearer ${token}` }, | ||
| }, | ||
| ); | ||
| return data; | ||
| }, | ||
|
|
||
| async checkPermission( | ||
| dto: CheckPermissionDto, | ||
| token: string, | ||
| ): Promise<PermissionCheckResponse> { | ||
| const { data } = await axios.post<PermissionCheckResponse>( | ||
| '/permissions/vm/check', | ||
| dto, | ||
| { | ||
| headers: { Authorization: `Bearer ${token}` }, | ||
| }, | ||
| ); | ||
| return data; | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify VM-specific batch and check endpoints
The VM API has batch creation and permission checking endpoints that the server API doesn't have. Please verify this asymmetry is intentional.
🌐 Web query:
Are the `/permissions/vm/batch` and `/permissions/vm/check` endpoints specific to VMs only, or should similar endpoints exist for server permissions?
💡 Result:
The /permissions/vm/batch and /permissions/vm/check endpoints, by their naming and general RBAC (Role-Based Access Control) design patterns, appear to be specific to virtual machine (VM) objects. This specificity is reflected in similar RBAC implementations, which associate permissions endpoints with particular object types—such as virtual machines, servers, folders, or services—depending on the needs of the platform[1][9].
Key reasons for this:
- Permissions APIs typically namespace endpoints by the object type. Permissions for different resource types (e.g., VMs, servers, folders) are handled by endpoints tailored to those types, with object type included in the path or parameters[1][9].
- Examples from industry documentation: In Puppet Enterprise, permissions are defined by object type (e.g.,
"vm","server","resource") and action; endpoints or payloads reference those types explicitly[1]. Similarly, in Red Hat Virtualization, you assign roles and permissions specifically to virtual machines, but you can also assign permissions to other resources such as data centers or clusters[9].
Should similar endpoints exist for server permissions?
- Yes. If your platform distinguishes between VMs and physical servers or other server-like objects, you should implement analogous endpoints (e.g.,
/permissions/server/batch,/permissions/server/check) to handle permission checks and updates for those resource types.- This separation improves clarity, maintainability, and auditing of permissions systems, aligning with best practices for RBAC API design[1][7].
- If your architecture uses a generic permissions endpoint (e.g.,
/permissions/{resourceType}/batch), you could parameterize the resource type to handle both VMs and servers in a consistent way[1][4].
Summary Table
| Endpoint Example | Object Type | Purpose |
|---|---|---|
/permissions/vm/batch |
Virtual Machine | Batch check/set VM permissions |
/permissions/vm/check |
Virtual Machine | Check VM permissions |
/permissions/server/batch |
Server (if exists) | Batch check/set server permissions |
/permissions/server/check |
Server (if exists) | Check server permissions |
In conclusion, the endpoints you mentioned are VM-specific, and similar endpoints should be implemented for other resource types (e.g., servers) as needed—unless your API is explicitly resource-agnostic and relies on parameters to specify the object type[1][9].
Citations:
- 1: https://www.puppet.com/docs/pe/2025.1/rbac_api_v1_permissions
- 2: https://www.altaro.com/vmware/using-permissions-to-secure-vcenter-server/
- 3: https://www.pluralsight.com/resources/blog/tech-operations/vmware-access-control-101-roles-and-permissions
- 4: https://endgrate.com/blog/role-based-access-control-for-apis-implementation-guide
- 5: https://learn.microsoft.com/en-us/azure/virtual-network/virtual-network-service-endpoints-overview
- 6: https://help.hcl-software.com/bigfix/11.0/inventory/Inventory/admin/t_set_perms_vmware.html
- 7: https://developers.arcgis.com/rest/enterprise-administration/server/permissions/
- 8: https://docs.skillable.com/docs/custom-vm-endpoints
- 9: https://docs.redhat.com/en/documentation/red_hat_virtualization/4.2/html/virtual_machine_management_guide/sect-virtual_machines_and_permissions
- 10: https://learn.microsoft.com/en-us/answers/questions/1015715/how-to-restrict-app-permissions-to-a-set-of-api-en
Implement Server-Specific Permission Endpoints
The /permissions/vm/batch and /permissions/vm/check routes you’ve added are indeed VM-specific. If your platform also manages permissions for physical servers (or other resource types), you should:
• In src/features/users/permission-api.ts (around lines 202–228), add methods analogous to createBatch and checkPermission, e.g.:
async createServerBatch(dto: BatchCreatePermissionDto, token: string): Promise<BatchPermissionResponse> { … axios.post('/permissions/server/batch', …) }async checkServerPermission(dto: CheckPermissionDto, token: string): Promise<PermissionCheckResponse> { … axios.post('/permissions/server/check', …) }
• Or, refactor both VM and server calls into a single parameterized method (e.g. /permissions/{resourceType}/{operation}) to avoid duplication.
This alignment with standard RBAC patterns will ensure clarity and maintainability across resource types.
🤖 Prompt for AI Agents
In src/features/users/permission-api.ts around lines 202 to 228, you need to add
methods for server-specific permission endpoints similar to the existing
VM-specific ones. Implement methods like createServerBatch and
checkServerPermission that post to '/permissions/server/batch' and
'/permissions/server/check' respectively, or refactor the existing methods to
accept a resourceType parameter to handle both VM and server permissions
dynamically, reducing code duplication and improving maintainability.
| const updatedUser = await updateUser( | ||
| props.user.id, | ||
| form.value, | ||
| authStore.token!, | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle missing auth token gracefully
Using non-null assertion on authStore.token! could throw a runtime error if the token is missing.
try {
+ if (!authStore.token) {
+ throw new Error('Authentication required');
+ }
const updatedUser = await updateUser(
props.user.id,
form.value,
- authStore.token!,
+ authStore.token,
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const updatedUser = await updateUser( | |
| props.user.id, | |
| form.value, | |
| authStore.token!, | |
| ); | |
| try { | |
| + if (!authStore.token) { | |
| + throw new Error('Authentication required'); | |
| + } | |
| const updatedUser = await updateUser( | |
| props.user.id, | |
| form.value, | |
| - authStore.token!, | |
| + authStore.token, | |
| ); |
🤖 Prompt for AI Agents
In src/features/users/components/UserEditRoleModal.vue around lines 76 to 80,
the code uses a non-null assertion on authStore.token!, which can cause a
runtime error if the token is missing. Modify the code to check if
authStore.token exists before calling updateUser, and handle the case where the
token is missing gracefully, such as by showing an error message or preventing
the update operation.
| const handleToggleUserStatus = async (user: User) => { | ||
| try { | ||
| await toggleUserStatus(user.id, authStore.token!); | ||
| loadUsers(page.value, pageSize); | ||
| closeActionsModal(); | ||
| } catch (err) {} | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add error handling for toggle user status
The empty catch block silently swallows errors, preventing users from knowing if the operation failed.
const handleToggleUserStatus = async (user: User) => {
try {
await toggleUserStatus(user.id, authStore.token!);
loadUsers(page.value, pageSize);
closeActionsModal();
- } catch (err) {}
+ } catch (err) {
+ showError(t('users.toggle_status_error'));
+ }
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleToggleUserStatus = async (user: User) => { | |
| try { | |
| await toggleUserStatus(user.id, authStore.token!); | |
| loadUsers(page.value, pageSize); | |
| closeActionsModal(); | |
| } catch (err) {} | |
| }; | |
| const handleToggleUserStatus = async (user: User) => { | |
| try { | |
| await toggleUserStatus(user.id, authStore.token!); | |
| loadUsers(page.value, pageSize); | |
| closeActionsModal(); | |
| } catch (err) { | |
| showError(t('users.toggle_status_error')); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In src/features/users/views/UserListView.vue around lines 130 to 136, the catch
block in the handleToggleUserStatus function is empty, which silently swallows
errors. Update the catch block to handle errors properly by logging the error or
displaying a user-friendly error message to inform users if the toggle operation
fails.
| "username": "Username", | ||
| "username_placeholder": "Enter username", | ||
| "email": "Email", | ||
| "email_placeholder": "user{'@'}example.com", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix malformed email placeholder
The email placeholder contains invalid syntax with curly braces.
- "email_placeholder": "user{'@'}example.com",
+ "email_placeholder": "[email protected]",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "email_placeholder": "user{'@'}example.com", | |
| "email_placeholder": "user@example.com", |
🤖 Prompt for AI Agents
In src/locales/en.json at line 808, the email placeholder value contains invalid
curly braces around the '@' symbol. Replace the curly braces with a plain '@'
character to correct the email format, changing "user{'@'}example.com" to
"[email protected]".
| ); | ||
|
|
||
| return { | ||
| id: isValidUUID ? w.id : crypto.randomUUID(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add fallback for crypto.randomUUID()
crypto.randomUUID() is not supported in older browsers or non-secure contexts.
Consider using a UUID library or implementing a fallback:
- id: isValidUUID ? w.id : crypto.randomUUID(),
+ id: isValidUUID ? w.id : (crypto.randomUUID?.() ?? generateFallbackUUID()),Add a fallback function:
function generateFallbackUUID(): string {
return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, (c) => {
const r = (Math.random() * 16) | 0;
const v = c === 'x' ? r : (r & 0x3) | 0x8;
return v.toString(16);
});
}🤖 Prompt for AI Agents
In src/features/dashboard/api.ts at line 46, the use of crypto.randomUUID()
lacks a fallback for environments where it is unsupported. Implement a fallback
UUID generator function using the provided pattern with Math.random, then modify
the code to use crypto.randomUUID() if available, otherwise use the fallback
function to generate the UUID.
| getStats: async (): Promise<FullDashboardStatsDto> => { | ||
| const { data } = await axios.get<FullDashboardStatsDto>( | ||
| '/dashboard/full', | ||
| getAuthHeaders(), | ||
| ); | ||
| return data; | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove duplicate method
The getStats method is an exact duplicate of getFullStats. This creates unnecessary code duplication.
Either remove this method or make it an alias:
- getStats: async (): Promise<FullDashboardStatsDto> => {
- const { data } = await axios.get<FullDashboardStatsDto>(
- '/dashboard/full',
- getAuthHeaders(),
- );
- return data;
- },
+ getStats: dashboardApi.getFullStats,Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/features/dashboard/api.ts around lines 71 to 77, the getStats method
duplicates the functionality of getFullStats, causing unnecessary code
duplication. Remove the getStats method entirely or replace it with an alias
that calls getFullStats to avoid redundancy while preserving functionality.
| try { | ||
| const response = await dashboardApi.getTemplates(); | ||
| templates.value = response.templates; | ||
| } catch {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid empty catch blocks
Another instance of error suppression that should be handled properly.
- } catch {}
+ } catch (error) {
+ console.error('Failed to load templates:', error);
+ templates.value = []; // Ensure templates is initialized
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch {} | |
| } catch (error) { | |
| console.error('Failed to load templates:', error); | |
| templates.value = []; // Ensure templates is initialized | |
| } |
🤖 Prompt for AI Agents
In src/features/dashboard/store.ts at line 170, the catch block is empty, which
suppresses errors silently. Modify the catch block to handle the error properly
by at least logging the error or taking appropriate recovery actions to avoid
silent failures.
| const loadPreferences = async () => { | ||
| try { | ||
| preferences.value = await dashboardApi.getPreferences(); | ||
| } catch {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid empty catch blocks
Empty catch blocks suppress errors silently, making debugging difficult.
- } catch {}
+ } catch (error) {
+ console.error('Failed to load preferences:', error);
+ // Return default preferences or handle gracefully
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch {} | |
| } catch (error) { | |
| console.error('Failed to load preferences:', error); | |
| // Return default preferences or handle gracefully | |
| } |
🤖 Prompt for AI Agents
In src/features/dashboard/store.ts at line 154, the catch block is empty, which
silently suppresses errors and hinders debugging. Modify the catch block to
handle the error properly by logging the error or rethrowing it, ensuring that
any exceptions are not ignored silently.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor
Chores