Refactor PersonalTableSettingsController and entity to handle nullable ordering fields #1511

Artuomka · 2026-01-20T13:34:19Z

No description provided.

…e ordering fields

Copilot

Pull request overview

This PR refactors the PersonalTableSettings entity and controller to support nullable ordering fields. The change allows users to have table settings without specifying ordering direction or ordering field values.

Changes:

Modified the ordering field in PersonalTableSettingsEntity to be nullable with a null default instead of required with 'ASC' default
Updated controller to remove the || null fallback for ordering and ordering_field when constructing table settings data
Modified the build utility function to preserve null values for ordering and ordering_field fields instead of deleting them

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
backend/src/migrations/1768915268048-MakeOrderingFieldInPersonalTableSettingsEntityNullable.ts	Database migration to make ordering column nullable and remove default value
backend/src/entities/table-settings/personal-table-settings/personal-table-settings.entity.ts	Updated entity definition with nullable ordering field and formatting changes
backend/src/entities/table-settings/personal-table-settings/personal-table-settings.controller.ts	Removed fallback logic for ordering fields to allow undefined values
backend/src/entities/table-settings/personal-table-settings/utils/build-new-personal-table-settings-entity.util.ts	Modified logic to preserve null values for nullable fields

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-20T13:38:11Z

backend/src/migrations/1768915268048-MakeOrderingFieldInPersonalTableSettingsEntityNullable.ts

+		await queryRunner.query(`ALTER TABLE "secret_access_logs" DROP CONSTRAINT "FK_secret_access_logs_secretId"`);
+		await queryRunner.query(`ALTER TABLE "secret_access_logs" DROP CONSTRAINT "FK_secret_access_logs_userId"`);
+		await queryRunner.query(`ALTER TABLE "user_secrets" DROP CONSTRAINT "FK_user_secrets_companyId"`);
+		await queryRunner.query(`DROP INDEX "public"."IDX_secret_access_logs_secretId"`);
+		await queryRunner.query(`DROP INDEX "public"."IDX_secret_access_logs_userId"`);
+		await queryRunner.query(`DROP INDEX "public"."IDX_secret_access_logs_accessedAt"`);
+		await queryRunner.query(`DROP INDEX "public"."IDX_user_secrets_companyId"`);
+		await queryRunner.query(`DROP INDEX "public"."IDX_user_secrets_createdAt"`);
+		await queryRunner.query(`DROP INDEX "public"."IDX_user_secrets_expiresAt"`);
+		await queryRunner.query(`DROP INDEX "public"."IDX_user_secrets_company_slug"`);
+		await queryRunner.query(`ALTER TABLE "personal_table_settings" ALTER COLUMN "ordering" DROP NOT NULL`);
+		await queryRunner.query(`ALTER TABLE "personal_table_settings" ALTER COLUMN "ordering" DROP DEFAULT`);
+		await queryRunner.query(`CREATE INDEX "IDX_69aeac5d0c61c697346fa2a0f8" ON "secret_access_logs" ("secretId") `);
+		await queryRunner.query(`CREATE INDEX "IDX_1d02f2dca9278e9a3925f9e797" ON "secret_access_logs" ("userId") `);
+		await queryRunner.query(`CREATE INDEX "IDX_ab33b550d45b31f76ac35a8c67" ON "secret_access_logs" ("accessedAt") `);
+		await queryRunner.query(`CREATE INDEX "IDX_8798678e66032251ff48185e96" ON "user_secrets" ("companyId") `);
+		await queryRunner.query(
+			`CREATE UNIQUE INDEX "IDX_f39a47aac503fe096b0c77f2b3" ON "user_secrets" ("companyId", "slug") `,
+		);
+		await queryRunner.query(
+			`ALTER TABLE "secret_access_logs" ADD CONSTRAINT "FK_69aeac5d0c61c697346fa2a0f83" FOREIGN KEY ("secretId") REFERENCES "user_secrets"("id") ON DELETE CASCADE ON UPDATE NO ACTION`,
+		);
+		await queryRunner.query(
+			`ALTER TABLE "secret_access_logs" ADD CONSTRAINT "FK_1d02f2dca9278e9a3925f9e797f" FOREIGN KEY ("userId") REFERENCES "user"("id") ON DELETE CASCADE ON UPDATE NO ACTION`,
+		);
+		await queryRunner.query(
+			`ALTER TABLE "user_secrets" ADD CONSTRAINT "FK_8798678e66032251ff48185e962" FOREIGN KEY ("companyId") REFERENCES "company_info"("id") ON DELETE CASCADE ON UPDATE NO ACTION`,
+		);
+	}
+
+	public async down(queryRunner: QueryRunner): Promise<void> {
+		await queryRunner.query(`ALTER TABLE "user_secrets" DROP CONSTRAINT "FK_8798678e66032251ff48185e962"`);
+		await queryRunner.query(`ALTER TABLE "secret_access_logs" DROP CONSTRAINT "FK_1d02f2dca9278e9a3925f9e797f"`);
+		await queryRunner.query(`ALTER TABLE "secret_access_logs" DROP CONSTRAINT "FK_69aeac5d0c61c697346fa2a0f83"`);
+		await queryRunner.query(`DROP INDEX "public"."IDX_f39a47aac503fe096b0c77f2b3"`);
+		await queryRunner.query(`DROP INDEX "public"."IDX_8798678e66032251ff48185e96"`);
+		await queryRunner.query(`DROP INDEX "public"."IDX_ab33b550d45b31f76ac35a8c67"`);


This migration contains substantial unrelated changes to secret_access_logs and user_secrets tables (lines 7-16, 19-34, 38-65) alongside the intended change to make the ordering field nullable in personal_table_settings (lines 17-18, 46-47). These unrelated changes appear to be recreating foreign keys and indexes with different names. Migrations should be focused on a single logical change. Please separate the personal_table_settings changes into their own migration.

Suggested change

await queryRunner.query(`ALTER TABLE "secret_access_logs" DROP CONSTRAINT "FK_secret_access_logs_secretId"`);

await queryRunner.query(`ALTER TABLE "secret_access_logs" DROP CONSTRAINT "FK_secret_access_logs_userId"`);

await queryRunner.query(`ALTER TABLE "user_secrets" DROP CONSTRAINT "FK_user_secrets_companyId"`);

await queryRunner.query(`DROP INDEX "public"."IDX_secret_access_logs_secretId"`);

await queryRunner.query(`DROP INDEX "public"."IDX_secret_access_logs_userId"`);

await queryRunner.query(`DROP INDEX "public"."IDX_secret_access_logs_accessedAt"`);

await queryRunner.query(`DROP INDEX "public"."IDX_user_secrets_companyId"`);

await queryRunner.query(`DROP INDEX "public"."IDX_user_secrets_createdAt"`);

await queryRunner.query(`DROP INDEX "public"."IDX_user_secrets_expiresAt"`);

await queryRunner.query(`DROP INDEX "public"."IDX_user_secrets_company_slug"`);

await queryRunner.query(`ALTER TABLE "personal_table_settings" ALTER COLUMN "ordering" DROP NOT NULL`);

await queryRunner.query(`ALTER TABLE "personal_table_settings" ALTER COLUMN "ordering" DROP DEFAULT`);

await queryRunner.query(`CREATE INDEX "IDX_69aeac5d0c61c697346fa2a0f8" ON "secret_access_logs" ("secretId") `);

await queryRunner.query(`CREATE INDEX "IDX_1d02f2dca9278e9a3925f9e797" ON "secret_access_logs" ("userId") `);

await queryRunner.query(`CREATE INDEX "IDX_ab33b550d45b31f76ac35a8c67" ON "secret_access_logs" ("accessedAt") `);

await queryRunner.query(`CREATE INDEX "IDX_8798678e66032251ff48185e96" ON "user_secrets" ("companyId") `);

await queryRunner.query(

`CREATE UNIQUE INDEX "IDX_f39a47aac503fe096b0c77f2b3" ON "user_secrets" ("companyId", "slug") `,

);

await queryRunner.query(

`ALTER TABLE "secret_access_logs" ADD CONSTRAINT "FK_69aeac5d0c61c697346fa2a0f83" FOREIGN KEY ("secretId") REFERENCES "user_secrets"("id") ON DELETE CASCADE ON UPDATE NO ACTION`,

);

await queryRunner.query(

`ALTER TABLE "secret_access_logs" ADD CONSTRAINT "FK_1d02f2dca9278e9a3925f9e797f" FOREIGN KEY ("userId") REFERENCES "user"("id") ON DELETE CASCADE ON UPDATE NO ACTION`,

);

await queryRunner.query(

`ALTER TABLE "user_secrets" ADD CONSTRAINT "FK_8798678e66032251ff48185e962" FOREIGN KEY ("companyId") REFERENCES "company_info"("id") ON DELETE CASCADE ON UPDATE NO ACTION`,

);

}

public async down(queryRunner: QueryRunner): Promise<void> {

await queryRunner.query(`ALTER TABLE "user_secrets" DROP CONSTRAINT "FK_8798678e66032251ff48185e962"`);

await queryRunner.query(`ALTER TABLE "secret_access_logs" DROP CONSTRAINT "FK_1d02f2dca9278e9a3925f9e797f"`);

await queryRunner.query(`ALTER TABLE "secret_access_logs" DROP CONSTRAINT "FK_69aeac5d0c61c697346fa2a0f83"`);

await queryRunner.query(`DROP INDEX "public"."IDX_f39a47aac503fe096b0c77f2b3"`);

await queryRunner.query(`DROP INDEX "public"."IDX_8798678e66032251ff48185e96"`);

await queryRunner.query(`DROP INDEX "public"."IDX_ab33b550d45b31f76ac35a8c67"`);

// Make the "ordering" field nullable and remove its default on personal_table_settings.

await queryRunner.query(`ALTER TABLE "personal_table_settings" ALTER COLUMN "ordering" DROP NOT NULL`);

await queryRunner.query(`ALTER TABLE "personal_table_settings" ALTER COLUMN "ordering" DROP DEFAULT`);

}

public async down(queryRunner: QueryRunner): Promise<void> {

// Roll back the change to the "ordering" field on personal_table_settings.

// (Any required ALTER TABLE statements to restore NOT NULL / DEFAULT remain below.)

Copilot · 2026-01-20T13:38:12Z

...able-settings/personal-table-settings/utils/build-new-personal-table-settings-entity.util.ts

 	Object.assign(newEntity, personalSettingsData);
 	Object.keys(personalSettingsData).forEach((key) => {
-		if (personalSettingsData[key as keyof PersonalTableSettingsData] === undefined) {
+		if (personalSettingsData[key as keyof PersonalTableSettingsData] === null && !NULLABLE_FIELDS.includes(key)) {


The logic change from checking for 'undefined' to checking for 'null' is problematic. The new condition deletes fields that are null unless they're in NULLABLE_FIELDS, but the previous logic deleted fields that were undefined. This means that fields explicitly set to null (except ordering and ordering_field) will now be deleted from the entity, which could cause unintended data loss. For instance, if a user explicitly sets columns_view to null to clear it, it would be deleted from the entity instead of being set to null in the database. Consider whether this behavior is intentional, and if so, ensure it's properly documented and tested.

Suggested change

if (personalSettingsData[key as keyof PersonalTableSettingsData] === null && !NULLABLE_FIELDS.includes(key)) {

if (personalSettingsData[key as keyof PersonalTableSettingsData] === undefined) {

Copilot · 2026-01-20T13:38:12Z

...nd/src/entities/table-settings/personal-table-settings/personal-table-settings.controller.ts

+				ordering: personalSettingsData.ordering,
+				ordering_field: personalSettingsData.ordering_field,


The removal of the '|| null' fallback for ordering and ordering_field creates a type inconsistency. The CreatePersonalTableSettingsDto fields are marked as @IsOptional, meaning they can be undefined when not provided in the request body. However, PersonalTableSettingsData type definition expects these fields to be 'QueryOrderingEnum | null' and 'string | null' respectively, not undefined. When these fields are not provided in the request, they will be undefined, which violates the type contract. Either keep the '|| null' fallback or update the PersonalTableSettingsData type to allow undefined values.

Suggested change

ordering: personalSettingsData.ordering,

ordering_field: personalSettingsData.ordering_field,

ordering: personalSettingsData.ordering || null,

ordering_field: personalSettingsData.ordering_field || null,

Copilot · 2026-01-20T13:38:12Z

backend/src/entities/table-settings/personal-table-settings/personal-table-settings.entity.ts

+	})
+	ordering!: QueryOrderingEnum;
+
+	@Column('varchar', { default: null })


The ordering_field column has 'default: null' but does not explicitly set 'nullable: true' in the column decorator. While TypeORM may infer nullability from the default value in some cases, it's best practice to be explicit, especially since the related 'ordering' field on line 15 explicitly sets 'nullable: true'. For consistency and clarity, add 'nullable: true' to the ordering_field column decorator options.

Suggested change

@Column('varchar', { default: null })

@Column('varchar', { nullable: true, default: null })

Refactor PersonalTableSettingsController and entity to handle nullabl…

98297da

…e ordering fields

Copilot AI review requested due to automatic review settings January 20, 2026 13:34

Artuomka merged commit cbc5281 into main Jan 20, 2026
17 checks passed

Artuomka deleted the backend_table_setting_ordering_cleaning branch January 20, 2026 13:34

Copilot started reviewing on behalf of Artuomka January 20, 2026 13:34 View session

Copilot AI reviewed Jan 20, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Refactor PersonalTableSettingsController and entity to handle nullable ordering fields #1511

Refactor PersonalTableSettingsController and entity to handle nullable ordering fields #1511

Uh oh!

Artuomka commented Jan 20, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	if (personalSettingsData[key as keyof PersonalTableSettingsData] === null && !NULLABLE_FIELDS.includes(key)) {
	if (personalSettingsData[key as keyof PersonalTableSettingsData] === undefined) {

		ordering: personalSettingsData.ordering,
		ordering_field: personalSettingsData.ordering_field,

	@Column('varchar', { default: null })
	@Column('varchar', { nullable: true, default: null })

Uh oh!

Refactor PersonalTableSettingsController and entity to handle nullable ordering fields #1511

Refactor PersonalTableSettingsController and entity to handle nullable ordering fields #1511

Uh oh!

Conversation

Artuomka commented Jan 20, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants