feat: update SavedDbQueryEntity and migration for widget properties management#1558
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the dashboard widget properties management by moving widget-related properties (widget_type, chart_type, widget_options) from the dashboard_widget table to the saved_db_query table. It also makes the query_text field nullable to support widget types (like Text and Counter) that don't require SQL queries.
Changes:
- Updated migration class name and timestamp to reflect the new migration version
- Modified SavedDbQueryEntity to add widget-related columns and make query_text nullable
- Reorganized database schema to centralize widget properties in saved_db_query
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| backend/src/migrations/1770126075082-MovedDashboardWidgetPropertiesToSavedQuery.ts | Renamed migration class, added widget property columns to saved_db_query, made query_text nullable, and dropped widget property columns from dashboard_widget |
| backend/src/entities/visualizations/saved-db-query/saved-db-query.entity.ts | Added widget_type, chart_type, and widget_options columns; made widget_type and query_text nullable with defaults |
Comments suppressed due to low confidence (3)
backend/src/migrations/1770126075082-MovedDashboardWidgetPropertiesToSavedQuery.ts:11
- The migration drops columns from dashboard_widget (including widget_type, name, description, chart_type, and widget_options) without migrating existing data to the saved_db_query table. If there are existing dashboard_widget records in the database, this data will be lost. Consider adding data migration logic to copy these values to their corresponding saved_db_query records before dropping the columns, or ensure this migration only runs on databases without existing data.
backend/src/migrations/1770126075082-MovedDashboardWidgetPropertiesToSavedQuery.ts:12 - For consistency with the entity definition, consider using DashboardWidgetTypeEnum.Chart instead of the string literal 'chart'. While both are functionally equivalent (the enum value is 'chart'), using the enum constant would maintain consistency with the entity decorator and make the code more maintainable if the enum value changes in the future.
backend/src/migrations/1770126075082-MovedDashboardWidgetPropertiesToSavedQuery.ts:19 - Setting query_text to NOT NULL during rollback may fail if there are saved_db_query records with null query_text values (e.g., for Text or Counter widget types that don't require queries). Consider either: (1) deleting or updating records with null query_text before setting NOT NULL, or (2) setting a default value for the column before adding the NOT NULL constraint.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| @Column({ type: 'text' }) | ||
| @Column({ type: 'text', nullable: true, default: null }) | ||
| query_text: string; |
There was a problem hiding this comment.
The TypeScript type for query_text is declared as non-nullable (string), but the column decorator marks it as nullable. This creates a type mismatch. The property type should be changed to 'string | null' to match the database schema and prevent potential runtime errors when accessing this field.
| query_text: string; | |
| query_text: string | null; |
|
|
||
| @Column({ type: 'varchar' }) | ||
| @Column({ type: 'varchar', nullable: true, default: DashboardWidgetTypeEnum.Chart }) | ||
| widget_type: DashboardWidgetTypeEnum; |
There was a problem hiding this comment.
The TypeScript type for widget_type is declared as non-nullable (DashboardWidgetTypeEnum), but the column decorator marks it as nullable with a default value. This creates a type mismatch. The property type should be changed to 'DashboardWidgetTypeEnum | null' to accurately reflect the database schema, or the nullable option should be removed if widget_type should always have a value.
| widget_type: DashboardWidgetTypeEnum; | |
| widget_type: DashboardWidgetTypeEnum | null; |
No description provided.