Skip to content

fix: update connection properties handling for table categories #1470

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
Dec 12, 2025
Merged

fix: update connection properties handling for table categories #1470

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
67 changes: 50 additions & 17 deletions ...end/src/entities/connection-properties/use-cases/update-connection-properties.use.case.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
} from '../utils/build-update-connection-properties-object.js';
import { validateCreateConnectionPropertiesDs } from '../utils/validate-create-connection-properties-ds.js';
import { IUpdateConnectionProperties } from './connection-properties-use.cases.interface.js';
import { TableCategoriesEntity } from '../../table-categories/table-categories.entity.js';

@Injectable()
export class UpdateConnectionPropertiesUseCase
Expand Down Expand Up @@ -46,30 +47,62 @@ export class UpdateConnectionPropertiesUseCase
const updatePropertiesObject: IUpdateConnectionPropertiesObject = buildUpdateConnectionPropertiesObject(inputData);
const updated = Object.assign(connectionPropertiesToUpdate, updatePropertiesObject);

const categoriesToRemove = await this._dbContext.tableCategoriesRepository.find({
const foundCategories = await this._dbContext.tableCategoriesRepository.find({
where: { connection_properties_id: connectionPropertiesToUpdate.id },
});

if (categoriesToRemove && categoriesToRemove.length > 0) {
await this._dbContext.tableCategoriesRepository.remove(categoriesToRemove);
updated.table_categories = [];
}
const newCategories: Array<TableCategoriesEntity> = [];

const updatedProperties = await this._dbContext.connectionPropertiesRepository.saveNewConnectionProperties(updated);
if (table_categories && table_categories.length) {
const createdCategories = table_categories.map((category) => {
const newCategory = this._dbContext.tableCategoriesRepository.create({
category_name: category.category_name,
tables: category.tables,
category_color: category.category_color,
category_id: category.category_id,
if (table_categories && table_categories.length > 0) {
const categoriesToRemove = foundCategories.filter((foundCategory) => {
return !table_categories?.some((inputCategory) => inputCategory.category_id === foundCategory.category_id);
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

The optional chaining operator on 'table_categories' is redundant here since this code is inside a block that already checks 'table_categories && table_categories.length > 0' at line 56. The variable is guaranteed to be defined at this point, so 'table_categories?.some' can be simplified to 'table_categories.some'.

Suggested change
return !table_categories?.some((inputCategory) => inputCategory.category_id === foundCategory.category_id);
return !table_categories.some((inputCategory) => inputCategory.category_id === foundCategory.category_id);

Copilot uses AI. Check for mistakes.
});
if (categoriesToRemove && categoriesToRemove.length > 0) {
await this._dbContext.tableCategoriesRepository.remove(categoriesToRemove);
}

const categoriesToCreate = table_categories.filter((inputCategory) => {
return !foundCategories.some((foundCategory) => foundCategory.category_id === inputCategory.category_id);
});

if (categoriesToCreate && categoriesToCreate.length > 0) {
const createdCategories = categoriesToCreate.map((category) => {
const newCategory = this._dbContext.tableCategoriesRepository.create({
category_name: category.category_name,
tables: category.tables,
category_color: category.category_color,
category_id: category.category_id,
});
newCategory.connection_properties = connectionPropertiesToUpdate;
return newCategory;
});
newCategory.connection_properties = updatedProperties;
return newCategory;
const savedNewCategories = await this._dbContext.tableCategoriesRepository.save(createdCategories);
newCategories.push(...savedNewCategories);
}

const categoriesToUpdate = table_categories.filter((inputCategory) => {
return foundCategories.some((foundCategory) => foundCategory.category_id === inputCategory.category_id);
});
const savedCategories = await this._dbContext.tableCategoriesRepository.save(createdCategories);
updatedProperties.table_categories = savedCategories;

for (const category of categoriesToUpdate) {
const categoryToUpdate = foundCategories.find(
(foundCategory) => foundCategory.category_id === category.category_id,
);
if (categoryToUpdate) {
categoryToUpdate.category_name = category.category_name;
categoryToUpdate.category_color = category.category_color;
categoryToUpdate.tables = category.tables;
const savedUpdatedCategory = await this._dbContext.tableCategoriesRepository.save(categoryToUpdate);
newCategories.push(savedUpdatedCategory);
}
}
Comment on lines +87 to +98
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

The update logic performs individual save operations in a loop, which results in N+1 database queries. Consider batching the updates using a single save call with an array of updated categories, similar to how new categories are created (line 79). This would reduce the number of database round-trips from N to 1.

Copilot uses AI. Check for mistakes.
} else {
newCategories.push(...foundCategories);
}
Comment on lines +56 to 101
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

The current logic doesn't distinguish between "not provided" (undefined/null) and "explicitly set to empty" (empty array). When table_categories is an empty array, the condition 'table_categories && table_categories.length > 0' evaluates to false, causing existing categories to be kept (line 100). This makes it impossible for API consumers to delete all categories by passing an empty array. Consider checking if table_categories is explicitly undefined/null versus an empty array to support both "keep existing" and "delete all" use cases.

Copilot uses AI. Check for mistakes.

const updatedProperties = await this._dbContext.connectionPropertiesRepository.saveNewConnectionProperties(updated);
updatedProperties.table_categories = newCategories;

return buildFoundConnectionPropertiesDs(updatedProperties);
}
}
99 changes: 92 additions & 7 deletions backend/test/ava-tests/saas-tests/connection-properties-e2e.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { ValidationException } from '../../../src/exceptions/custom-exceptions/v
import { ValidationError } from 'class-validator';
import { Cacher } from '../../../src/helpers/cache/cacher.js';
import { WinstonLogger } from '../../../src/entities/logging/winston-logger.js';
import { CreateTableCategoryDto } from '../../../src/entities/table-categories/dto/create-table-category.dto.js';

const mockFactory = new MockFactory();
let app: INestApplication;
Expand Down Expand Up @@ -273,7 +274,7 @@ test.serial(`${currentTest} should return created connection properties with tab
updateConnectionPropertiesROWithoutCategories.default_showing_table,
updatedConnectionPropertiesWithOutCategories.default_showing_table,
);
t.is(updateConnectionPropertiesROWithoutCategories.table_categories.length, 0);
t.is(updateConnectionPropertiesROWithoutCategories.table_categories.length, 1);
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

The test comment at line 243 states "should delete categories on update with empty categories" but the test no longer verifies this behavior. After this change, the test expects 1 category to remain (line 277) instead of 0, which contradicts the comment's intent. Either the test needs to be updated to match the new behavior (keeping categories when not provided), or the comment needs to be updated to reflect the new expectation.

Copilot uses AI. Check for mistakes.
} catch (e) {
throw e;
}
Expand Down Expand Up @@ -623,9 +624,93 @@ test.serial(`${currentTest} should return updated connection properties`, async
}
});

// test.serial(`${currentTest} `, async (t) => {
// try {
// } catch (e) {
// throw e;
// }
// });
test.serial(
`${currentTest} should keep created table categories if the exists return updated connection properties`,
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

The test description contains a grammatical error. The phrase "if the exists" appears to be missing a word. It should likely be "if they exist" or similar phrasing.

Suggested change
`${currentTest} should keep created table categories if the exists return updated connection properties`,
`${currentTest} should keep created table categories if they exist return updated connection properties`,

Copilot uses AI. Check for mistakes.
async (t) => {
try {
const { newConnection2, newConnectionToTestDB, updateConnection, newGroup1, newConnection } = getTestData();
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

Unused variable newConnection2.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

Unused variable newConnectionToTestDB.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

Unused variable updateConnection.

Suggested change
const { newConnection2, newConnectionToTestDB, updateConnection, newGroup1, newConnection } = getTestData();
const { newConnection2, newConnectionToTestDB, newGroup1, newConnection } = getTestData();

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

Unused variable newGroup1.

Copilot uses AI. Check for mistakes.
const { token } = await registerUserAndReturnUserInfo(app);
const secondHiddenTableName = `${faker.lorem.words(1)}_${faker.string.uuid()}`;
await resetPostgresTestDB(secondHiddenTableName);

const createConnectionResponse = await request(app.getHttpServer())
.post('/connection')
.send(newConnection)
.set('Cookie', token)
.set('Content-Type', 'application/json')
.set('Accept', 'application/json');
const createConnectionRO = JSON.parse(createConnectionResponse.text);
t.is(createConnectionResponse.status, 201);

const createConnectionPropertiesResponse = await request(app.getHttpServer())
.post(`/connection/properties/${createConnectionRO.id}`)
.send(newConnectionProperties)
.set('Cookie', token)
.set('Content-Type', 'application/json')
.set('Accept', 'application/json');
const createConnectionPropertiesRO = JSON.parse(createConnectionPropertiesResponse.text);
t.is(createConnectionPropertiesResponse.status, 201);
t.is(createConnectionPropertiesRO.hidden_tables[0], newConnectionProperties.hidden_tables[0]);
t.is(createConnectionPropertiesRO.connectionId, createConnectionRO.id);

const categoriesDTO: Array<CreateTableCategoryDto> = [
{
category_name: 'Category 1',
category_color: '#FF5733',
tables: [testTableName],
category_id: 'cat-001',
},
];

const createTableCategoriesResponse = await request(app.getHttpServer())
.put(`/table-categories/${createConnectionRO.id}`)
.send(categoriesDTO)
.set('Cookie', token)
.set('Content-Type', 'application/json')
.set('Accept', 'application/json');

const createTableCategoriesRO = JSON.parse(createTableCategoriesResponse.text);

t.is(createTableCategoriesResponse.status, 200);
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

The variable 'createTableCategoriesRO' is assigned but never used in the test. Consider either removing this variable assignment or adding assertions to verify the response content from the table categories creation endpoint.

Suggested change
t.is(createTableCategoriesResponse.status, 200);
t.is(createTableCategoriesResponse.status, 200);
// Assert the response content for table categories creation
t.true(Array.isArray(createTableCategoriesRO));
t.is(createTableCategoriesRO.length, 1);
t.is(createTableCategoriesRO[0].category_name, 'Category 1');
t.is(createTableCategoriesRO[0].category_color, '#FF5733');
t.deepEqual(createTableCategoriesRO[0].tables, [testTableName]);
t.is(createTableCategoriesRO[0].category_id, 'cat-001');

Copilot uses AI. Check for mistakes.

const propertiesCopy = JSON.parse(JSON.stringify(newConnectionProperties));
propertiesCopy.hidden_tables = [testTableName, secondHiddenTableName];

const updateConnectionPropertiesResponse = await request(app.getHttpServer())
.put(`/connection/properties/${createConnectionRO.id}`)
.send(propertiesCopy)
.set('Cookie', token)
.set('Content-Type', 'application/json')
.set('Accept', 'application/json');

t.is(updateConnectionPropertiesResponse.status, 200);

const updateConnectionPropertiesRO = JSON.parse(updateConnectionPropertiesResponse.text);
t.is(updateConnectionPropertiesRO.hidden_tables.length, 2);
t.is(updateConnectionPropertiesRO.hidden_tables.includes(testTableName), true);
t.is(updateConnectionPropertiesRO.hidden_tables.includes(secondHiddenTableName), true);
t.is(updateConnectionPropertiesRO.connectionId, createConnectionRO.id);
t.is(updateConnectionPropertiesRO.table_categories.length, 1);
t.is(updateConnectionPropertiesRO.table_categories[0].category_name, 'Category 1');
t.is(updateConnectionPropertiesRO.table_categories[0].tables.length, 1);
t.is(updateConnectionPropertiesRO.table_categories[0].tables[0], testTableName);

const findTableCategoriesResponse = await request(app.getHttpServer())
.get(`/table-categories/${createConnectionRO.id}`)
.set('Cookie', token)
.set('Content-Type', 'application/json')
.set('Accept', 'application/json');

const findTableCategoriesRO = JSON.parse(findTableCategoriesResponse.text);

t.is(findTableCategoriesResponse.status, 200);

t.is(findTableCategoriesRO.length, 1);
t.is(findTableCategoriesRO[0].category_name, 'Category 1');
t.is(findTableCategoriesRO[0].tables.length, 1);
t.is(findTableCategoriesRO[0].tables[0], testTableName);
} catch (e) {
throw e;
}
},
);
Loading