Skip to content

Refactor PersonalTableSettingsController and entity to handle nullable ordering fields #1511

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Artuomka merged 1 commit into from
Jan 20, 2026
Merged

Refactor PersonalTableSettingsController and entity to handle nullable ordering fields #1511

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ...src/entities/table-settings/personal-table-settings/personal-table-settings.controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ export class PersonalTableSettingsController {
columns_view: personalSettingsData.columns_view || null,
list_fields: personalSettingsData.list_fields || null,
list_per_page: personalSettingsData.list_per_page || null,
ordering: personalSettingsData.ordering || null,
ordering_field: personalSettingsData.ordering_field || null,
ordering: personalSettingsData.ordering,
ordering_field: personalSettingsData.ordering_field,
Comment on lines +119 to +120
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
original_names: personalSettingsData.original_names || null,
},
};
Expand Down
98 changes: 53 additions & 45 deletions ...end/src/entities/table-settings/personal-table-settings/personal-table-settings.entity.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,49 +5,57 @@ import { UserEntity } from '../../user/user.entity.js';

@Entity('personal_table_settings')
export class PersonalTableSettingsEntity {
@PrimaryGeneratedColumn('uuid')
id: string;

@Column({ default: null })
table_name: string;

@Column('enum', {
nullable: false,
enum: QueryOrderingEnum,
default: QueryOrderingEnum.ASC,
})
ordering!: QueryOrderingEnum;

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

@Column('int', { default: null })
list_per_page: number;

@Column('varchar', { array: true, default: {} })
list_fields: string[];

@Column({ type: 'varchar', array: true, default: {} })
columns_view: Array<string>;

@Column('boolean', { default: false })
original_names: boolean;

@ManyToOne((_) => ConnectionEntity, (connection) => connection.personal_table_settings, {
onDelete: 'CASCADE',
})
@JoinColumn({ name: 'connection_id' })
connection: Relation<ConnectionEntity>;

@Column()
connection_id: string;

@ManyToOne((_) => UserEntity, (user) => user.personal_table_settings, {
onDelete: 'CASCADE',
})
@JoinColumn({ name: 'user_id' })
user: Relation<UserEntity>;

@Column()
user_id: string;
@PrimaryGeneratedColumn('uuid')
id: string;

@Column({ default: null })
table_name: string;

@Column('enum', {
nullable: true,
enum: QueryOrderingEnum,
default: null,
})
ordering!: QueryOrderingEnum;

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

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

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 })

Copilot uses AI. Check for mistakes.
ordering_field: string;

@Column('int', { default: null })
list_per_page: number;

@Column('varchar', { array: true, default: {} })
list_fields: string[];

@Column({ type: 'varchar', array: true, default: {} })
columns_view: Array<string>;

@Column('boolean', { default: false })
original_names: boolean;

@ManyToOne(
(_) => ConnectionEntity,
(connection) => connection.personal_table_settings,
{
onDelete: 'CASCADE',
},
)
@JoinColumn({ name: 'connection_id' })
connection: Relation<ConnectionEntity>;

@Column()
connection_id: string;

@ManyToOne(
(_) => UserEntity,
(user) => user.personal_table_settings,
{
onDelete: 'CASCADE',
},
)
@JoinColumn({ name: 'user_id' })
user: Relation<UserEntity>;

@Column()
user_id: string;
}
4 changes: 3 additions & 1 deletion ...e-settings/personal-table-settings/utils/build-new-personal-table-settings-entity.util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import { PersonalTableSettingsData } from '../data-structures/create-personal-table-settings.ds.js';
import { PersonalTableSettingsEntity } from '../personal-table-settings.entity.js';

const NULLABLE_FIELDS = ['ordering', 'ordering_field'];

export function buildNewPersonalTableSettingsEntity(
personalSettingsData: PersonalTableSettingsData,
): PersonalTableSettingsEntity {
const newEntity = new PersonalTableSettingsEntity();
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)) {
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
// eslint-disable-next-line security/detect-object-injection
delete (newEntity as any)[key];
}
Expand Down
67 changes: 67 additions & 0 deletions ...nd/src/migrations/1768915268048-MakeOrderingFieldInPersonalTableSettingsEntityNullable.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import { MigrationInterface, QueryRunner } from 'typeorm';

export class MakeOrderingFieldInPersonalTableSettingsEntityNullable1768915268048 implements MigrationInterface {
name = 'MakeOrderingFieldInPersonalTableSettingsEntityNullable1768915268048';

public async up(queryRunner: QueryRunner): Promise<void> {
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"`);
Comment on lines +7 to +43
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
await queryRunner.query(`DROP INDEX "public"."IDX_1d02f2dca9278e9a3925f9e797"`);
await queryRunner.query(`DROP INDEX "public"."IDX_69aeac5d0c61c697346fa2a0f8"`);
await queryRunner.query(`ALTER TABLE "personal_table_settings" ALTER COLUMN "ordering" SET DEFAULT 'ASC'`);
await queryRunner.query(`ALTER TABLE "personal_table_settings" ALTER COLUMN "ordering" SET NOT NULL`);
await queryRunner.query(
`CREATE UNIQUE INDEX "IDX_user_secrets_company_slug" ON "user_secrets" ("companyId", "slug") `,
);
await queryRunner.query(`CREATE INDEX "IDX_user_secrets_expiresAt" ON "user_secrets" ("expiresAt") `);
await queryRunner.query(`CREATE INDEX "IDX_user_secrets_createdAt" ON "user_secrets" ("createdAt") `);
await queryRunner.query(`CREATE INDEX "IDX_user_secrets_companyId" ON "user_secrets" ("companyId") `);
await queryRunner.query(`CREATE INDEX "IDX_secret_access_logs_accessedAt" ON "secret_access_logs" ("accessedAt") `);
await queryRunner.query(`CREATE INDEX "IDX_secret_access_logs_userId" ON "secret_access_logs" ("userId") `);
await queryRunner.query(`CREATE INDEX "IDX_secret_access_logs_secretId" ON "secret_access_logs" ("secretId") `);
await queryRunner.query(
`ALTER TABLE "user_secrets" ADD CONSTRAINT "FK_user_secrets_companyId" FOREIGN KEY ("companyId") REFERENCES "company_info"("id") ON DELETE CASCADE ON UPDATE NO ACTION`,
);
await queryRunner.query(
`ALTER TABLE "secret_access_logs" ADD CONSTRAINT "FK_secret_access_logs_userId" FOREIGN KEY ("userId") REFERENCES "user"("id") ON DELETE CASCADE ON UPDATE NO ACTION`,
);
await queryRunner.query(
`ALTER TABLE "secret_access_logs" ADD CONSTRAINT "FK_secret_access_logs_secretId" FOREIGN KEY ("secretId") REFERENCES "user_secrets"("id") ON DELETE CASCADE ON UPDATE NO ACTION`,
);
}
}
Loading