table settings: restore order_by, column order and visible by default columns #1517
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request restores previously commented-out table settings features related to column ordering, visibility, and default sort order. The changes enable users to configure which columns are visible by default, customize column order through drag-and-drop, and set default ordering for table views.
Changes:
- Added new properties to TableSettings interface:
ordering,ordering_field,list_fields, andcolumns_view - Restored UI controls for managing visible columns, column order, and default sorting
- Implemented fallback logic to use personal view settings when table settings are not configured
- Added drag-and-drop functionality for reordering columns with state tracking
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| frontend/src/app/models/table.ts | Added four new required properties to TableSettings interface for storing column order, visible columns, and sorting preferences |
| frontend/src/app/components/dashboard/db-tables-data-source.ts | Added fallback logic to check personal view settings (res.columns_view) when table settings columns_view is not available |
| frontend/src/app/components/dashboard/db-table-view/db-table-settings/db-table-settings.component.ts | Implemented drag-and-drop handlers, order tracking flag, and initialization logic for managing column order state |
| frontend/src/app/components/dashboard/db-table-view/db-table-settings/db-table-settings.component.html | Uncommented form controls for column visibility, drag-and-drop column ordering, and default sort configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (key === 'list_fields') { | ||
| updatedSettings[key] = this.orderChanged; | ||
| } else { | ||
| updatedSettings[key] = value.length > 0; | ||
| } |
There was a problem hiding this comment.
The analytics tracking for list_fields is inconsistent with other array fields. While other array fields like search_fields and excluded_fields track whether they have values (value.length > 0), list_fields tracks the orderChanged flag. This inconsistency means that for analytics purposes, if list_fields has values but wasn't changed in this session, it would be tracked as false, while an empty search_fields array would also be tracked as false. This makes it difficult to interpret the analytics data consistently.
| this.listFieldsOrder = [...res.list_fields]; | ||
| } else { | ||
| this.tableSettings = this.tableSettingsInitial; | ||
| }; | ||
| if (Object.keys(res).length === 0 || (res && res.list_fields && !res.list_fields.length)) { |
There was a problem hiding this comment.
The initialization of listFieldsOrder at line 146 will fail when res.list_fields is undefined or null, as spreading undefined/null will throw a runtime error. The check at line 150 only handles empty arrays, not the case where list_fields doesn't exist on the response object. This can occur when table settings exist but don't have list_fields populated, which would cause a crash when trying to spread the undefined value.
| this.listFieldsOrder = [...res.list_fields]; | |
| } else { | |
| this.tableSettings = this.tableSettingsInitial; | |
| }; | |
| if (Object.keys(res).length === 0 || (res && res.list_fields && !res.list_fields.length)) { | |
| this.listFieldsOrder = Array.isArray(res.list_fields) ? [...res.list_fields] : []; | |
| } else { | |
| this.tableSettings = this.tableSettingsInitial; | |
| }; | |
| if (Object.keys(res).length === 0 || !res.list_fields || !res.list_fields.length) { |
…rocket-admin/rocketadmin into table-settings-restore-settings
No description provided.