Skip to content

enable sorting for all table category #1620

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 2 commits into from
Feb 20, 2026
Merged

enable sorting for all table category #1620

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dbf25e4
enable sorting for all table category
Artuomka Feb 20, 2026
9e026d8
Merge branch 'main' into backend_table_categories_sorting_fix
Artuomka Feb 20, 2026
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
24 changes: 9 additions & 15 deletions ...end/src/entities/table-categories/use-cases/create-or-update-table-categories.use.case.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ export class CreateOrUpdateTableCategoriesUseCase
protected async implementation(inputData: CreateOrUpdateTableCategoriesDS): Promise<Array<FoundTableCategoryRo>> {
const { connectionId, master_password, table_categories } = inputData;

const allTablesCategory = table_categories.find((category) => category.category_id === null);
const filteredCategories = table_categories.filter((category) => category.category_id !== null);
const allTablesCategory = table_categories.find((category) => category.category_id === 'all-tables-kitten');
const filteredCategories = table_categories.filter((category) => category.category_id !== 'all-tables-kitten');

const foundConnection = await this._dbContext.connectionRepository.findAndDecryptConnection(
connectionId,
Expand All @@ -49,7 +49,12 @@ export class CreateOrUpdateTableCategoriesUseCase
connectionProperties = await this._dbContext.connectionPropertiesRepository.save(newConnectionProperties);
}

const createdCategories = filteredCategories.map((category) => {
const categoriesToSave = [...filteredCategories];
if (allTablesCategory) {

Copilot AI Feb 20, 2026

Copy link

Choose a reason for hiding this comment

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

The all-tables-kitten category bypasses validation (line 34 validates only filteredCategories). This means invalid table names could be stored in all-tables-kitten without validation. While the find use case filters out non-existent tables at runtime, consider whether basic validation (non-empty category_name, tables as array) should still apply to all-tables-kitten for data integrity.

Suggested change
if (allTablesCategory) {
if (allTablesCategory) {
if (
typeof allTablesCategory.category_name !== 'string' ||
allTablesCategory.category_name.trim().length === 0 ||
!Array.isArray(allTablesCategory.tables)
) {
throw new Error('Invalid all-tables-kitten category payload');
}

Copilot uses AI. Check for mistakes.
categoriesToSave.unshift(allTablesCategory);
}

const createdCategories = categoriesToSave.map((category) => {
const newCategory = this._dbContext.tableCategoriesRepository.create({
category_name: category.category_name,
tables: category.tables,
Expand All @@ -60,17 +65,6 @@ export class CreateOrUpdateTableCategoriesUseCase
return newCategory;
});
const savedCategories = await this._dbContext.tableCategoriesRepository.save(createdCategories);
const result = savedCategories.map((category) => buildFoundTableCategoryRo(category));

if (allTablesCategory) {
result.unshift({
category_id: null,
category_name: allTablesCategory.category_name,
category_color: allTablesCategory.category_color,
tables: allTablesCategory.tables,
});
}

return result;
return savedCategories.map((category) => buildFoundTableCategoryRo(category));
}
}
33 changes: 24 additions & 9 deletions ...end/src/entities/table-categories/use-cases/find-table-categories-with-tables.use.case.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,19 +90,34 @@ export class FindTableCategoriesWithTablesUseCase
const foundTableCategories =
await this._dbContext.tableCategoriesRepository.findTableCategoriesForConnection(connectionId);

// const sortedTables = tablesRO.sort((tableRO1, tableRO2) => {
// const name1 = tableRO1.display_name || tableRO1.table;
// const name2 = tableRO2.display_name || tableRO2.table;
// return name1.localeCompare(name2);
// });
const storedAllTablesCategory = foundTableCategories.find((category) => category.category_id === 'all-tables-kitten');
const otherCategories = foundTableCategories.filter((category) => category.category_id !== 'all-tables-kitten');

let allTablesOrdered: Array<FoundTableDs>;
if (storedAllTablesCategory) {
allTablesOrdered = storedAllTablesCategory.tables
.map((tableName) => tablesRO.find((tableRO) => tableRO.table === tableName))
.filter(Boolean);

Copilot AI Feb 20, 2026

Copy link

Choose a reason for hiding this comment

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

The filter(Boolean) operation is potentially unsafe in TypeScript because the type system doesn't correctly narrow Array<FoundTableDs | undefined> to Array<FoundTableDs>. While it works at runtime, it would be more type-safe to use .filter((table): table is FoundTableDs => table !== undefined). This also applies to similar logic in lines 121-123 for other categories.

Suggested change
.filter(Boolean);
.filter((table): table is FoundTableDs => table !== undefined);

Copilot uses AI. Check for mistakes.

const storedTableNames = new Set(storedAllTablesCategory.tables);
const newTables = tablesRO.filter((tableRO) => !storedTableNames.has(tableRO.table));

if (newTables.length > 0) {
allTablesOrdered = [...allTablesOrdered, ...newTables];
storedAllTablesCategory.tables = allTablesOrdered.map((t) => t.table);
await this._dbContext.tableCategoriesRepository.save(storedAllTablesCategory);
}
Comment on lines +97 to +109

Copilot AI Feb 20, 2026

Copy link

Choose a reason for hiding this comment

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

The logic handles adding new tables but does not handle removing deleted tables from the stored order. If a table is deleted from the database but still exists in storedAllTablesCategory.tables, it will be filtered out by filter(Boolean) in line 100, but the database will not be updated to reflect this change. This can lead to the stored order containing references to non-existent tables that accumulate over time. Consider updating the stored order in the database when tables are removed, similar to how it's done when new tables are added.

Copilot uses AI. Check for mistakes.
} else {
allTablesOrdered = tablesRO;
}

const allTableCategory: FoundTableCategoriesWithTablesRo = {
category_id: null,
category_color: null,
category_id: 'all-tables-kitten',

Copilot AI Feb 20, 2026

Copy link

Choose a reason for hiding this comment

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

The identifier 'all-tables-kitten' is an unusual naming choice for a system-level constant. Consider using a more conventional identifier like 'all-tables', 'all', or '_all_tables' that better reflects its special system status. The '-kitten' suffix appears arbitrary and may confuse developers unfamiliar with the codebase context.

Copilot uses AI. Check for mistakes.
Comment on lines +93 to +115

Copilot AI Feb 20, 2026

Copy link

Choose a reason for hiding this comment

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

The magic string 'all-tables-kitten' is duplicated across multiple files (in this file at lines 93, 94, 115, and in create-or-update-table-categories.use.case.ts at lines 26, 27). Consider extracting this to a shared constant to ensure consistency and make future changes easier. For example, define it as a constant like ALL_TABLES_CATEGORY_ID in a shared constants file.

Copilot uses AI. Check for mistakes.

Copilot AI Feb 20, 2026

Copy link

Choose a reason for hiding this comment

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

This changes the API contract for the v2 endpoint where category_id was previously null for the "All tables" category and is now 'all-tables-kitten'. This is a breaking change for any API consumers expecting null. Consider whether this should be versioned as v3 or if there's documentation/migration guide needed for existing API consumers.

Suggested change
category_id: 'all-tables-kitten',
category_id: null,

Copilot uses AI. Check for mistakes.
category_color: storedAllTablesCategory?.category_color ?? null,
category_name: 'All tables',

Copilot AI Feb 20, 2026

Copy link

Choose a reason for hiding this comment

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

The category_name is hardcoded to 'All tables' regardless of what name is stored in the database for the all-tables-kitten category. This means if a user creates an all-tables-kitten category with a different name, it will be silently changed to 'All tables' in the response. Consider whether this behavior should validate and enforce the name during creation, or document this override behavior clearly.

Suggested change
category_name: 'All tables',
category_name: storedAllTablesCategory?.category_name ?? 'All tables',

Copilot uses AI. Check for mistakes.
tables: tablesRO,
tables: allTablesOrdered,
};
const foundTableCategoriesRO = foundTableCategories.map((category) => {
const foundTableCategoriesRO = otherCategories.map((category) => {
const tablesInCategory = category.tables
.map((tableName) => tablesRO.find((tableRO) => tableRO.table === tableName))
.filter(Boolean);
Expand Down
148 changes: 147 additions & 1 deletion backend/test/ava-tests/saas-tests/table-categories-e2e.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ test.serial(`${currentTest} find table categories with tables`, async (t) => {
t.is(findTableCategoriesWithTablesResponse.status, 200);

t.is(findTableCategoriesWithTablesRO[0].category_name, 'All tables');
t.is(findTableCategoriesWithTablesRO[0].category_id, null);
t.is(findTableCategoriesWithTablesRO[0].category_id, 'all-tables-kitten');
t.is(findTableCategoriesWithTablesRO[0].category_color, null);
t.true(findTableCategoriesWithTablesRO[0].tables.length >= 2);

Expand Down Expand Up @@ -389,6 +389,152 @@ test.serial(`${currentTest} find table categories with tables`, async (t) => {
}
});

test.serial(`${currentTest} should store all-tables-kitten and preserve its order in v2 response`, async (t) => {
try {
const connectionToTestDB = getTestData(mockFactory).connectionToPostgres;
const firstUserToken = (await registerUserAndReturnUserInfo(app)).token;
const { testTableName: firstTestTableName } = await createTestTable(connectionToTestDB);
const { testTableName: secondTestTableName } = await createTestTable(connectionToTestDB);

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

// create all-tables-kitten with intentional order: second, first
const categoriesDTO: Array<CreateTableCategoryDto> = [
{
category_name: 'All tables',
category_color: null,
tables: [secondTestTableName, firstTestTableName],
category_id: 'all-tables-kitten',
},
{
category_name: 'Category 1',
category_color: '#FF5733',
tables: [firstTestTableName],
category_id: 'cat-001',
},
];

const createResponse = await request(app.getHttpServer())
.put(`/table-categories/${createConnectionRO.id}`)
.send(categoriesDTO)
.set('Cookie', firstUserToken)
.set('Content-Type', 'application/json')
.set('Accept', 'application/json');
t.is(createResponse.status, 200);
const createRO = JSON.parse(createResponse.text);

// all-tables-kitten should be first in the response
t.is(createRO[0].category_id, 'all-tables-kitten');
t.deepEqual(createRO[0].tables, [secondTestTableName, firstTestTableName]);
t.is(createRO[1].category_id, 'cat-001');

// v2 GET should use stored order from all-tables-kitten
const findResponse = await request(app.getHttpServer())
.get(`/table-categories/v2/${createConnectionRO.id}`)
.set('Cookie', firstUserToken)
.set('Content-Type', 'application/json')
.set('Accept', 'application/json');
const findRO = JSON.parse(findResponse.text);
t.is(findResponse.status, 200);

t.is(findRO[0].category_id, 'all-tables-kitten');
t.is(findRO[0].category_name, 'All tables');
// stored order should be preserved: second, first
const allTablesTableNames = findRO[0].tables.map((t) => t.table);
const secondIdx = allTablesTableNames.indexOf(secondTestTableName);
const firstIdx = allTablesTableNames.indexOf(firstTestTableName);
t.true(secondIdx < firstIdx);
} catch (e) {
console.error(e);
t.fail();
}
});

test.serial(`${currentTest} should append new tables at the end of all-tables-kitten`, async (t) => {
try {
const connectionToTestDB = getTestData(mockFactory).connectionToPostgres;
const firstUserToken = (await registerUserAndReturnUserInfo(app)).token;
const { testTableName: firstTestTableName } = await createTestTable(connectionToTestDB);
const { testTableName: secondTestTableName } = await createTestTable(connectionToTestDB);

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

// create all-tables-kitten with only the first two tables (second, first order)
const categoriesDTO: Array<CreateTableCategoryDto> = [
{
category_name: 'All tables',
category_color: null,
tables: [secondTestTableName, firstTestTableName],
category_id: 'all-tables-kitten',
},
];

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

// now create a third table in the database (simulates a new table appearing)
const { testTableName: thirdTestTableName } = await createTestTable(connectionToTestDB);

// v2 GET should detect the new table and append it at the end
const findResponse = await request(app.getHttpServer())
.get(`/table-categories/v2/${createConnectionRO.id}`)
.set('Cookie', firstUserToken)
.set('Content-Type', 'application/json')
.set('Accept', 'application/json');
const findRO = JSON.parse(findResponse.text);
t.is(findResponse.status, 200);

const allTablesCategory = findRO[0];
t.is(allTablesCategory.category_id, 'all-tables-kitten');
const allTablesTableNames = allTablesCategory.tables.map((t) => t.table);

// original order preserved: second before first
const secondIdx = allTablesTableNames.indexOf(secondTestTableName);
const firstIdx = allTablesTableNames.indexOf(firstTestTableName);
const thirdIdx = allTablesTableNames.indexOf(thirdTestTableName);
t.true(secondIdx < firstIdx);
// new table should be appended after both existing tables
t.true(thirdIdx > firstIdx);
t.truthy(thirdIdx >= 0);

// second GET should still have the same order (updated in DB)
const findResponse2 = await request(app.getHttpServer())
.get(`/table-categories/v2/${createConnectionRO.id}`)
.set('Cookie', firstUserToken)
.set('Content-Type', 'application/json')
.set('Accept', 'application/json');
const findRO2 = JSON.parse(findResponse2.text);
const allTablesTableNames2 = findRO2[0].tables.map((t) => t.table);
const secondIdx2 = allTablesTableNames2.indexOf(secondTestTableName);
const firstIdx2 = allTablesTableNames2.indexOf(firstTestTableName);
const thirdIdx2 = allTablesTableNames2.indexOf(thirdTestTableName);
t.true(secondIdx2 < firstIdx2);
t.true(thirdIdx2 > firstIdx2);
} catch (e) {
console.error(e);
t.fail();
}
});

test.serial(`${currentTest} should preserve table order from database in category response`, async (t) => {
try {
const connectionToTestDB = getTestData(mockFactory).connectionToPostgres;
Expand Down
Loading