Skip to content

Add new fields and enum for table settings: list_fields, list_per_page, ordering, and ordering_field#1512

Merged
Artuomka merged 1 commit into
mainfrom
backend_table_settings_rework
Jan 21, 2026
Merged

Add new fields and enum for table settings: list_fields, list_per_page, ordering, and ordering_field#1512
Artuomka merged 1 commit into
mainfrom
backend_table_settings_rework

Conversation

@Artuomka

Copy link
Copy Markdown
Collaborator

No description provided.

Copilot AI review requested due to automatic review settings January 21, 2026 09:36
@Artuomka Artuomka enabled auto-merge January 21, 2026 09:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 tableSettings table with appropriate types and constraints
  • Updated TableSettingsEntity to include the new fields with proper TypeORM decorators
  • Modified buildDAOsTableSettingsDs to 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,

Copilot AI Jan 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
list_per_page: personalTableSettings?.list_per_page || commonTableSettings?.list_per_page,
list_per_page: personalTableSettings?.list_per_page ?? commonTableSettings?.list_per_page,

Copilot uses AI. Check for mistakes.
Comment on lines +41 to +45
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,

Copilot AI Jan 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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,

Copilot uses AI. Check for mistakes.
@Artuomka Artuomka merged commit 690d34d into main Jan 21, 2026
25 checks passed
@Artuomka Artuomka deleted the backend_table_settings_rework branch January 21, 2026 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants