Add new fields and enum for table settings: list_fields, list_per_page, ordering, and ordering_field#1512
Conversation
…e, ordering, and ordering_field
There was a problem hiding this comment.
Pull request overview
This pull request restores four fields (list_fields, list_per_page, ordering, and ordering_field) to the common table settings entity that were previously moved to personal table settings only. This allows these fields to serve as defaults at the connection level, with personal settings able to override them. The builder function has been updated to implement a fallback pattern where personal settings take precedence over common settings.
Changes:
- Added database migration to restore four fields to the
tableSettingstable with appropriate types and constraints - Updated
TableSettingsEntityto include the new fields with proper TypeORM decorators - Modified
buildDAOsTableSettingsDsto implement fallback logic from personal to common settings - Applied consistent tab-based formatting throughout the affected files
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| backend/src/migrations/1768987695763-RestoreFieldsInCommonTableSettings.ts | Adds migration to restore list_fields, list_per_page, ordering, and ordering_field columns to tableSettings table with proper up/down migrations |
| backend/src/entities/table-settings/common-table-settings/table-settings.entity.ts | Adds four new fields to TableSettingsEntity with appropriate TypeORM decorators and imports QueryOrderingEnum |
| shared-code/src/helpers/data-structures-builders/table-settings.ds.builder.ts | Updates type definitions and implements fallback logic for new fields in the builder function |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| excluded_fields: commonTableSettings?.excluded_fields, | ||
| list_fields: personalTableSettings?.list_fields || commonTableSettings?.list_fields || [], | ||
| identification_fields: commonTableSettings?.identification_fields, | ||
| list_per_page: personalTableSettings?.list_per_page || commonTableSettings?.list_per_page, |
There was a problem hiding this comment.
The OR operator will treat 0 as falsy, which could be a valid value for list_per_page (meaning no pagination or 0 items per page). If a user sets personalTableSettings.list_per_page to 0, it will incorrectly fall back to commonTableSettings.list_per_page. Consider using nullish coalescing operator (??) instead of logical OR (||) to properly handle 0 as a valid value.
| list_per_page: personalTableSettings?.list_per_page || commonTableSettings?.list_per_page, | |
| list_per_page: personalTableSettings?.list_per_page ?? commonTableSettings?.list_per_page, |
| list_fields: personalTableSettings?.list_fields || commonTableSettings?.list_fields || [], | ||
| identification_fields: commonTableSettings?.identification_fields, | ||
| list_per_page: personalTableSettings?.list_per_page || commonTableSettings?.list_per_page, | ||
| ordering: personalTableSettings?.ordering || commonTableSettings?.ordering, | ||
| ordering_field: personalTableSettings?.ordering_field || commonTableSettings?.ordering_field, |
There was a problem hiding this comment.
The OR operator will treat empty strings as falsy values. If ordering_field is set to an empty string in personalTableSettings, it will incorrectly fall back to commonTableSettings.ordering_field instead of using the empty string. Consider using nullish coalescing operator (??) instead of logical OR (||) to properly distinguish between empty strings and null/undefined values.
| list_fields: personalTableSettings?.list_fields || commonTableSettings?.list_fields || [], | |
| identification_fields: commonTableSettings?.identification_fields, | |
| list_per_page: personalTableSettings?.list_per_page || commonTableSettings?.list_per_page, | |
| ordering: personalTableSettings?.ordering || commonTableSettings?.ordering, | |
| ordering_field: personalTableSettings?.ordering_field || commonTableSettings?.ordering_field, | |
| list_fields: personalTableSettings?.list_fields ?? commonTableSettings?.list_fields ?? [], | |
| identification_fields: commonTableSettings?.identification_fields, | |
| list_per_page: personalTableSettings?.list_per_page ?? commonTableSettings?.list_per_page, | |
| ordering: personalTableSettings?.ordering ?? commonTableSettings?.ordering, | |
| ordering_field: personalTableSettings?.ordering_field ?? commonTableSettings?.ordering_field, |
No description provided.