fix: update connection properties handling for table categories #1470

Artuomka · 2025-12-12T18:36:42Z

No description provided.

Copilot

Pull request overview

This PR updates the connection properties handling logic to preserve existing table categories when they are not explicitly provided in update requests, rather than deleting them. The key change is a shift from a "replace all" approach to a more granular "create/update/delete" approach that allows categories to persist across property updates.

Key changes:

Modified the update use case to selectively create, update, or remove table categories based on category_id matching
Changed behavior to keep existing categories when table_categories field is not provided or empty
Added comprehensive test coverage for the new "keep existing categories" behavior

Reviewed changes

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

File	Description
backend/src/entities/connection-properties/use-cases/update-connection-properties.use.case.ts	Refactored category handling logic from "replace all" to "selective create/update/delete" with category_id-based matching
backend/test/ava-tests/saas-tests/connection-properties-e2e.test.ts	Updated existing test expectation from 0 to 1 categories and added new test case verifying categories persist across property updates

Comments suppressed due to low confidence (1)

backend/test/ava-tests/saas-tests/connection-properties-e2e.test.ts:672

Unused variable createTableCategoriesRO.

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

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

Copilot · 2025-12-12T18:40:47Z

backend/test/ava-tests/saas-tests/connection-properties-e2e.test.ts

-//   }
-// });
+test.serial(
+  `${currentTest} should keep created table categories if the exists return updated connection properties`,


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 · 2025-12-12T18:40:48Z

backend/test/ava-tests/saas-tests/connection-properties-e2e.test.ts

+
+      const createTableCategoriesRO = JSON.parse(createTableCategoriesResponse.text);
+
+      t.is(createTableCategoriesResponse.status, 200);


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 · 2025-12-12T18:40:48Z

backend/src/entities/connection-properties/use-cases/update-connection-properties.use.case.ts

+    if (table_categories && table_categories.length > 0) {
+      const categoriesToRemove = foundCategories.filter((foundCategory) => {
+        return !table_categories?.some((inputCategory) => inputCategory.category_id === foundCategory.category_id);
+      });
+      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);
+        }
+      }
+    } else {
+      newCategories.push(...foundCategories);
    }


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 · 2025-12-12T18:40:48Z

backend/test/ava-tests/saas-tests/connection-properties-e2e.test.ts

      updatedConnectionPropertiesWithOutCategories.default_showing_table,
    );
-    t.is(updateConnectionPropertiesROWithoutCategories.table_categories.length, 0);
+    t.is(updateConnectionPropertiesROWithoutCategories.table_categories.length, 1);


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 · 2025-12-12T18:40:48Z

backend/src/entities/connection-properties/use-cases/update-connection-properties.use.case.ts

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


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 · 2025-12-12T18:40:49Z

backend/src/entities/connection-properties/use-cases/update-connection-properties.use.case.ts

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


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 · 2025-12-12T18:40:49Z

backend/test/ava-tests/saas-tests/connection-properties-e2e.test.ts

+  `${currentTest} should keep created table categories if the exists return updated connection properties`,
+  async (t) => {
+    try {
+      const { newConnection2, newConnectionToTestDB, updateConnection, newGroup1, newConnection } = getTestData();


Unused variable newConnection2.

Copilot · 2025-12-12T18:40:49Z

backend/test/ava-tests/saas-tests/connection-properties-e2e.test.ts

+  `${currentTest} should keep created table categories if the exists return updated connection properties`,
+  async (t) => {
+    try {
+      const { newConnection2, newConnectionToTestDB, updateConnection, newGroup1, newConnection } = getTestData();


Unused variable newConnectionToTestDB.

Copilot · 2025-12-12T18:40:49Z

backend/test/ava-tests/saas-tests/connection-properties-e2e.test.ts

+  `${currentTest} should keep created table categories if the exists return updated connection properties`,
+  async (t) => {
+    try {
+      const { newConnection2, newConnectionToTestDB, updateConnection, newGroup1, newConnection } = getTestData();


Unused variable updateConnection.

Suggested change

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

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

Copilot · 2025-12-12T18:40:50Z

backend/test/ava-tests/saas-tests/connection-properties-e2e.test.ts

+  `${currentTest} should keep created table categories if the exists return updated connection properties`,
+  async (t) => {
+    try {
+      const { newConnection2, newConnectionToTestDB, updateConnection, newGroup1, newConnection } = getTestData();


Unused variable newGroup1.

fix: update connection properties handling for table categories

6855e01

Copilot AI review requested due to automatic review settings December 12, 2025 18:36

Copilot started reviewing on behalf of Artuomka December 12, 2025 18:37 View session

Artuomka enabled auto-merge December 12, 2025 18:37

Copilot AI reviewed Dec 12, 2025

View reviewed changes

Artuomka merged commit c7f4752 into main Dec 12, 2025
24 checks passed

Artuomka deleted the backend_table_categories_fix branch December 12, 2025 18:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix: update connection properties handling for table categories #1470

fix: update connection properties handling for table categories #1470

Artuomka commented Dec 12, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	`${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`,


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

		t.is(createTableCategoriesResponse.status, 200);

-      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');

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

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

Uh oh!

fix: update connection properties handling for table categories #1470

fix: update connection properties handling for table categories #1470

Conversation

Artuomka commented Dec 12, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants