feat: add dashboard core - #1797
Conversation
a6d2848 to
9a820c3
Compare
| return userSettings; | ||
| const widgetIds = userSettings.dashboard.widgets.map((widget) => widget.id); | ||
| const validIds = await this.filterDashboardWidgets(userId, widgetIds); | ||
| const returnedWidgets = userSettings.dashboard.widgets.filter((widget) => |
There was a problem hiding this comment.
what if there is no dashboard in users settings jsonb yet?
There was a problem hiding this comment.
Created a migration to ensure consistent data in db and resolve potentional issues with lack of dashboard property in user settings
| const { roleSlugs } = await this.permissionsService.getUserAccess(userId); | ||
| const userRoles = new Set(roleSlugs); | ||
| const globalSettings = await this.getPublicGlobalSettings(); |
There was a problem hiding this comment.
@its-gabo shouldn't the dbInstance be passed down to those awaited methods?
There was a problem hiding this comment.
There’s no need to do that, unless it’s part of a transaction and we specifically need the state of the data within given transaction.
If the concern was that it wouldn’t work because of RLS, that shouldn’t be an issue. The call happens within the request context, so the data remains properly scoped through AsyncLocalStorage.
| return new BaseResponse(await this.settingsService.updateUserSettings(userId, updatedSettings)); | ||
| } | ||
|
|
||
| @Get("dashboard") |
There was a problem hiding this comment.
shouldn't be guarded with @RequirePermission(PERMISSIONS.DASHBOARD_READ)?
There was a problem hiding this comment.
Added missing Permission decorator to dashboard endpoints
| widgets: [ | ||
| { | ||
| id: DASHBOARD_WIDGET_IDS.ADMIN_PLACEHOLDER1, | ||
| enabled: true, |
There was a problem hiding this comment.
is this flag used anywhere but specs?
239cd70 to
306e711
Compare
|
@its-gabo please review |
its-gabo
left a comment
There was a problem hiding this comment.
Pretty good in overall, just some few minor things to adjust
| queryFn: async () => { | ||
| try { | ||
| const response = await ApiClient.api.settingsControllerGetAvailableDashboardWidgets(); | ||
|
|
||
| return response.data; | ||
| } catch (error) { | ||
| if (isAxiosError(error) && error.response?.status === 401) { | ||
| return null; | ||
| } | ||
| throw error; | ||
| } | ||
| }, | ||
| staleTime: 1000 * 60 * 5, | ||
| }); |
There was a problem hiding this comment.
Is the try .. catch needed here? Can we just not query when there's no session for a user?
| if (!data) { | ||
| throw new Error("User not authenticated"); | ||
| } |
There was a problem hiding this comment.
Translate the error please
| queryFn: async () => { | ||
| try { | ||
| const response = await ApiClient.api.settingsControllerGetDefaultDashboardWidgets(); | ||
|
|
||
| return response.data; | ||
| } catch (error) { | ||
| if (isAxiosError(error) && error.response?.status === 401) { | ||
| return null; | ||
| } | ||
| throw error; | ||
| } | ||
| }, | ||
| staleTime: 1000 * 60 * 5, | ||
| }); |
| if (!data) { | ||
| throw new Error("User not authenticated"); | ||
| } |
|
|
||
| const OVERLAY_MODIFIERS = [restrictOverlayToViewport]; | ||
|
|
||
| function SortableWidget({ widget, isEditing, onWidthChange }: SortableWidgetProps) { |
There was a problem hiding this comment.
Let's extract this component to a separate file, alongside the types
| @@ -1,5 +1,5 @@ | |||
| /* AUTO-GENERATED FILE - DO NOT EDIT BY HAND */ | |||
| const titleKey = "dashboardHome.widgets.a_placeholder_3.title"; | ||
| const descriptionKey = "dashboardHome.widgets.placeholderDescription"; | ||
| const iconClassName = "text-warning-700"; | ||
| const iconContainerClassName = "bg-warning-50"; |
There was a problem hiding this comment.
Is there a reason for having them not in the html? (applies to all placeholder widget files)
| export const adminPlaceholder3Widget = { | ||
| component: WidgetAdminPlaceholder3, | ||
| titleKey, | ||
| descriptionKey, | ||
| icon: ListChecks, | ||
| iconClassName, | ||
| iconContainerClassName, | ||
| } satisfies DashboardWidgetModule; |
There was a problem hiding this comment.
Can we move all of them to one file and then import these configs and use them in components?
So then the consts from the top of the file can be deleted and we can use it like adminPlaceholder3Widget.icon to display icon for example
(applies to all)
its-gabo
left a comment
There was a problem hiding this comment.
Looks a bit overcomplicated, is there a way to eliminate some of the refs perhaps?
| ...updatedSettings, | ||
| dashboard: { | ||
| widgets: [...submittedWidgets] | ||
| .sort((first, second) => first.order - second.order) |
There was a problem hiding this comment.
Sorting feels kind of redundant since FE already sorts by order
| import type { UpdateUserSettingsBody, UpdateUserSettingsResponse } from "../generated-api"; | ||
| import type { UseMutationResult } from "@tanstack/react-query"; | ||
|
|
||
| export function useUpdateDashboardWidgets(): UseMutationResult< |
There was a problem hiding this comment.
Do we need to type this explicitly?
| height: number; | ||
| }; | ||
|
|
||
| type DropPlacement = "before" | "after"; |
There was a problem hiding this comment.
Since we use these values several times here, might as well extract it to an enum and derive the union from it
There was a problem hiding this comment.
So much is happening here, let's extract reusable utilities to separate file, merge refs into one ref with object.
| const displayedWidgets = previewWidgets ?? sortedWidgets; | ||
| const activeEntry = activeId ? DASHBOARD_WIDGET_REGISTRY[activeId] : null; | ||
| const ActiveIcon = activeEntry?.icon; | ||
| const dashboardCollisionDetection: CollisionDetection = (args) => { |
There was a problem hiding this comment.
The logic is very abstract, can we include some jsdoc to explain what is happening in each function?
There was a problem hiding this comment.
Either rename extension to .ts or change name to WidgetRegistry.tsx
44a1e85 to
095ed57
Compare
095ed57 to
dd67618
Compare
Issue(s)
Overview
Adds a customizable dashboard with role-based widgets. Users can reorder entire widget cards, resize supported widgets, manage visibility, restore defaults, and save their layout.
Business Value
Provides users with a personalized dashboard focused on relevant information and activities. The modular widget structure also makes it easier to add dedicated business widgets without changing the dashboard layout system.
Screenshots / Video