#5645 - Institution profile updates - DB Migrations and Lookup data API by dheepak-aot · Pull Request #5691 · bcgov/SIMS

dheepak-aot · 2026-01-30T16:47:48Z

Institution profile updates - DB Migrations and Lookup data API

DB Migrations

Migration 1: DB migration to create the system-user if the system-user is not already present in DB.
Need for creating this migration: When insert scripts are executed as part of DB migrations, we require an audit-user as creator of inserted records and the audit-user can only be a system user in this case. But when these scripts execute on a clean DB, say E2E test database then there is no system-user available for the DB migrations because it is created only during nest application startup which happens after DB Migrations are completed. Hence we added a migration to create a system-user if the same does not already exist in the DB.
Migration 2: DB Migration to create the table sims.system_lookup_configurations to configure the lookup data under different lookup categories. This lookup service is a generic solution which is primarily created to act as a store/API to provide values to web clients that are required to load the components.
Migration 3: DB Migration to create the STABLE function sims.is_valid_system_lookup_key which can be used in a check constraint for columns in tables which is based on the lookup data key. For e.g. sims.institutions has the column country which has country code value from the sims.system_lookup_configurations under lookup category Country.
Migration 4: DB Migration to add new columns to sims.institutions and update one or more new columns based on the existing institution types.

Rollback Evidence

System lookup Module and API

Created service which loads lookup data on module start-up and a controller SystemLookupConfigurationController that is accessible to all clients (except external) similar to dynamic form configuration controller.

API E2E Tests

Added E2E tests to for the SystemLookupConfigurationController.

SystemLookupConfigurationController(e2e)-getSystemLookupEntriesByCategory
    √ Should get the system lookup test entries for the category country when one ore more test entries exist for the same category. (168 ms)                                                     
    √ Should throw bad error when the lookup category is invalid. (47 ms)

…service

Copilot

Pull request overview

This PR adds institution profile fields and introduces a system lookup configuration mechanism to store reference data like countries and provinces. The changes include database migrations, entity models, a new service layer for lookup management, and an API controller for accessing lookup data.

Changes:

Added system lookup configuration table with countries and provinces reference data
Extended institution table with new profile fields (country, province, classification, organization status, medical school status)
Created service layer to load and cache lookup configurations in memory
Added API controller endpoint to retrieve lookup entries by category

Reviewed changes

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

Show a summary per file

File	Description
system-lookup-configuration.model.ts	New entity model for storing lookup data by category
system-lookup-category.type.ts	Enum defining lookup categories (Country, Province)
institution*.type.ts	Three new enums for institution classification, organization status, and medical school status
institution.model.ts	Added five nullable columns for institution profile information
Add-additional-information-cols.sql	Migration adds institution columns with validation constraints and backfills existing data
Create-system-lookup-configurations-table.sql	Creates lookup table with unique constraint on category+key
Insert-country-and-province-system-lookup.sql	Seeds 250+ countries and 13 Canadian provinces/territories
Create-function-is-valid-system-lookup-key.sql	PostgreSQL function to validate lookup keys via CHECK constraints
system-lookup-configuration.service.ts	Service to load and cache lookup configurations in memory
system-lookup-configuration.module.ts	Global module that loads lookups on application startup
system-lookup-configuration.controller.ts	Shared controller endpoint for all client types
system-lookup-configuration.dto.ts	Output DTOs for lookup entries
system-lookup-configuration.controller.getSystemLookupEntriesByCategory.e2e-spec.ts	E2E tests for the lookup endpoint
createFakeSystemLookupConfiguration	Test factory for creating fake lookup entries
create-institutions-and-authentication-users.ts	Updated test seeding to include new institution fields

Copilot · 2026-02-03T00:58:51Z

.../backend/libs/services/src/system-lookup-configuration/system-lookup-configuration.module.ts

+   */
+  async onModuleInit(): Promise<void> {
+    this.logger.log("Loading system lookup configurations.");
+    await this.systemLookupConfigurationService.loadAllSystemLookupConfigurations();


The service loads all system lookup configurations into memory on module initialization without any error handling. If the database query fails during startup, the application may start with an empty lookup map, potentially causing runtime errors. Consider adding error handling and logging for the initial load operation.

Suggested change

await this.systemLookupConfigurationService.loadAllSystemLookupConfigurations();

try {

await this.systemLookupConfigurationService.loadAllSystemLookupConfigurations();

this.logger.log("System lookup configurations loaded successfully.");

} catch (error: unknown) {

if (error instanceof Error) {

this.logger.error(

`Failed to load system lookup configurations during module initialization. ${error.message}`,

error.stack,

);

} else {

this.logger.error(

"Failed to load system lookup configurations during module initialization due to an unknown error.",

);

}

throw error;

}

@dheepak-aot Are we not adding this suggested try-catch block?

By adding the try-catch as described above, only possible benefit is custom error logging, does it justify the update?

...backend/libs/services/src/system-lookup-configuration/system-lookup-configuration.service.ts

.../src/route-controllers/system-lookup-configuration/models/system-lookup-configuration.dto.ts

...backend/libs/services/src/system-lookup-configuration/system-lookup-configuration.service.ts

.../backend/libs/services/src/system-lookup-configuration/system-lookup-configuration.module.ts

Copilot · 2026-02-03T00:58:53Z

sources/packages/backend/libs/sims-db/src/entities/institution.model.ts

+  // TODO: These additional columns are currently nullable until all existing institutions
+  // have been backfilled with appropriate data.
+  // After that, these columns should not be nullable.


The comment incorrectly states these columns will be made non-nullable after backfilling, but the migration already includes an UPDATE statement that backfills existing institutions with values based on their institution type. The TODO comment should clarify whether additional manual backfilling is needed for edge cases not covered by the UPDATE statement, or if this TODO is obsolete.

Suggested change

// TODO: These additional columns are currently nullable until all existing institutions

// have been backfilled with appropriate data.

// After that, these columns should not be nullable.

// These additional columns are kept nullable in the entity to remain compatible

// with existing data and the current database schema. Any change to make them

// non-nullable must be done explicitly in a dedicated migration coordinated with

// data backfilling and business requirements.

...backend/libs/services/src/system-lookup-configuration/system-lookup-configuration.service.ts

.../src/route-controllers/system-lookup-configuration/models/system-lookup-configuration.dto.ts

.../src/route-controllers/system-lookup-configuration/system-lookup-configuration.controller.ts

...sts_/e2e/system-lookup-configuration.controller.getSystemLookupEntriesByCategory.e2e-spec.ts

andrewsignori-aot · 2026-02-03T20:24:01Z

.../packages/backend/apps/db-migrations/src/sql/Institution/Add-additional-information-cols.sql

+            -- System user.
+            user_name = '8fb44f70-6ce6-11ed-b307-8743a2da47ef@system'
+    )
+WHERE


I do not understand the benefit of having the check between 1 and 7. IMO, it would be equivalent to check IF EXISTS, which we stopped doing because if something is outside the expectation and an error happens, the error should happen.

andrewsignori-aot · 2026-02-03T20:27:43Z

sources/packages/backend/apps/test-db-seeding/src/clean-db/clean-db.ts

    await this.dataSource.query(
      "DROP FUNCTION IF EXISTS sims.create_history_entry",
    );
+    await this.dataSource.query(


Minor, I do not recall if there was a reason not call all these DB commands in parallel.
It can be a quick improvement (outside PR scope).

andrewsignori-aot · 2026-02-03T20:28:25Z

sources/packages/backend/apps/test-db-seeding/src/clean-db/clean-db.ts

      "DROP FUNCTION IF EXISTS sims.create_history_entry",
    );
+    await this.dataSource.query(
+      "DROP FUNCTION IF EXISTS sims.is_valid_system_lookup_key(TEXT,TEXT)",


Do we need the (TEXT,TEXT) at the end since line 13 does not have it?

As discussed, the function with parameters requires it.

andrewsignori-aot · 2026-02-03T21:26:52Z

...sts_/e2e/system-lookup-configuration.controller.getSystemLookupEntriesByCategory.e2e-spec.ts

+  });
+  beforeEach(async () => {
+    // Clean up the system lookup configuration test entries before each test.
+    await db.systemLookupConfiguration.delete({ lookupKey: ILike("TEST_%") });


Minor, the TEST_ can be a const.

andrewsignori-aot · 2026-02-03T21:30:04Z

...sts_/e2e/system-lookup-configuration.controller.getSystemLookupEntriesByCategory.e2e-spec.ts

+  it("Should get the system lookup test entries for the category country when one or more test entries exist for the same category.", async () => {
+    // Arrange
+    // Create test system lookup entries for country.
+    const testCountryLookup1 = createFakeSystemLookupConfiguration(


Minor and not a blocker can be simplified as below.

const [testCountryLookup1, testCountryLookup2] = Array.from({ length: 2, }).map((_, index) => createFakeSystemLookupConfiguration( { auditUser, }, { initialValues: { lookupCategory: SystemLookupCategory.Country, lookupKey: `TEST_CN${index}`, lookupValue: `Test Country${index}`, }, }, ), );

andrewsignori-aot

Amazing work with the system lookup, the check constraint, and the DB seeding adjustments. Only minor and non-blocker comments, hence approving.

sh16011993 · 2026-02-03T23:47:07Z

...sts_/e2e/system-lookup-configuration.controller.getSystemLookupEntriesByCategory.e2e-spec.ts

+    const testCountryLookup2 = createFakeSystemLookupConfiguration(
+      {
+        auditUser,
+      },
+      {
+        initialValues: {
+          lookupCategory: SystemLookupCategory.Country,
+          lookupKey: "TEST_CN2",
+          lookupValue: "Test Country2",
+        },
+      },
+    );


Can we add a 3rd lookup category, anything other than a country (say Province) testing the part that it should not get returned.

Adding 3rd lookup category was my first thought, but it requires updating the enum with 3rd category SystemLookupCategory.Some3rdCategory for the API to allow it. so i took this approach.

It could have been tested with the existing Province category.

sh16011993

LGTM @dheepak-aot 👍

sonarqubecloud · 2026-02-04T03:33:16Z

Quality Gate passed

Issues
8 New issues
1 Accepted issue

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

github-actions · 2026-02-04T03:33:53Z

Backend Unit Tests Coverage Report

Totals
Statements:	20.28% ( 4361 / 21503 )
Methods:	9.8% ( 257 / 2623 )
Lines:	24.43% ( 3738 / 15298 )
Branches:	10.22% ( 366 / 3582 )

github-actions · 2026-02-04T03:34:11Z

E2E Workflow Workers Coverage Report

Totals
Statements:	75.41% ( 1055 / 1399 )
Methods:	79.31% ( 115 / 145 )
Lines:	78.79% ( 769 / 976 )
Branches:	61.51% ( 171 / 278 )

github-actions · 2026-02-04T03:37:16Z

E2E Queue Consumers Coverage Report

Totals
Statements:	85.68% ( 1616 / 1886 )
Methods:	85% ( 187 / 220 )
Lines:	88.64% ( 1287 / 1452 )
Branches:	66.36% ( 142 / 214 )

github-actions · 2026-02-04T03:38:03Z

E2E SIMS API Coverage Report

Totals
Statements:	77.29% ( 8877 / 11486 )
Methods:	76.81% ( 1050 / 1367 )
Lines:	81.32% ( 6443 / 7923 )
Branches:	63.02% ( 1384 / 2196 )

dheepak-aot added 7 commits January 28, 2026 18:12

DB Migrations

c161de9

Adding system lookup configurations sql script

b5edf51

Added All migrations.

632d9c9

Merge branch 'main' into updates/#5645-institution-profile-updates-1

b6f0af6

reverting the camunda file

fb617c9

reverting the camunda file

7bb4a28

System lookup configuration service

22152e6

dheepak-aot self-assigned this Jan 30, 2026

dheepak-aot added the DB DB migration involved label Jan 30, 2026

dheepak-aot added 4 commits January 30, 2026 17:31

Adding types for new columns

222fd22

Function cleanup

b7eaed8

Add DB migration to have system user setup upfront.

207c7b2

Added module controller and E2E test for system lookup configuration …

49b9b84

…service

dheepak-aot requested a review from Copilot February 3, 2026 00:54

Copilot started reviewing on behalf of dheepak-aot February 3, 2026 00:54 View session

Copilot AI reviewed Feb 3, 2026

View reviewed changes

dheepak-aot added 3 commits February 2, 2026 18:02

Updated query with where clause.

b0f10ae

updated as per copilot

8054c92

added jsdoc comments.

705cc40

dheepak-aot changed the title ~~#5645 - Institution profile updates 1~~ #5645 - Institution profile updates 1 - DB Migrations and Lookup data API Feb 3, 2026

dheepak-aot changed the title ~~#5645 - Institution profile updates 1 - DB Migrations and Lookup data API~~ #5645 - Institution profile updates - DB Migrations and Lookup data API Feb 3, 2026

dheepak-aot added SIMS-Api SIMS-Api E2E/Unit tests labels Feb 3, 2026

dheepak-aot marked this pull request as ready for review February 3, 2026 06:48

Adding sort to ensure consistent order of results.

d1072cc

sh16011993 self-requested a review February 3, 2026 17:16

andrewsignori-aot self-requested a review February 3, 2026 17:34

andrewsignori-aot reviewed Feb 3, 2026

View reviewed changes

andrewsignori-aot approved these changes Feb 3, 2026

View reviewed changes

sh16011993 reviewed Feb 3, 2026

View reviewed changes

sh16011993 approved these changes Feb 3, 2026

View reviewed changes

Code review changes

5d44305

dheepak-aot added this pull request to the merge queue Feb 4, 2026

Merged via the queue into main with commit b6958f4 Feb 4, 2026
22 checks passed

dheepak-aot deleted the updates/#5645-institution-profile-updates-1 branch February 4, 2026 17:00

dheepak-aot temporarily deployed to DEV February 4, 2026 17:05 — with GitHub Actions Inactive

dheepak-aot temporarily deployed to DEV February 4, 2026 17:09 — with GitHub Actions Inactive

dheepak-aot restored the updates/#5645-institution-profile-updates-1 branch February 4, 2026 20:27

dheepak-aot deleted the updates/#5645-institution-profile-updates-1 branch February 4, 2026 20:27

-    await this.systemLookupConfigurationService.loadAllSystemLookupConfigurations();
+    try {
+      await this.systemLookupConfigurationService.loadAllSystemLookupConfigurations();
+      this.logger.log("System lookup configurations loaded successfully.");
+    } catch (error: unknown) {
+      if (error instanceof Error) {
+        this.logger.error(
+          `Failed to load system lookup configurations during module initialization. ${error.message}`,
+          error.stack,
+        );
+      } else {
+        this.logger.error(
+          "Failed to load system lookup configurations during module initialization due to an unknown error.",
+        );
+      }
+      throw error;
+    }

-  // TODO: These additional columns are currently nullable until all existing institutions
-  // have been backfilled with appropriate data.
-  // After that, these columns should not be nullable.
+  // These additional columns are kept nullable in the entity to remain compatible
+  // with existing data and the current database schema. Any change to make them
+  // non-nullable must be done explicitly in a dedicated migration coordinated with
+  // data backfilling and business requirements.

Conversation

dheepak-aot commented Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Institution profile updates - DB Migrations and Lookup data API

DB Migrations

Rollback Evidence

System lookup Module and API

API E2E Tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewsignori-aot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sh16011993 left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Quality Gate passed

Uh oh!

github-actions bot commented Feb 4, 2026

Backend Unit Tests Coverage Report

Uh oh!

github-actions bot commented Feb 4, 2026

E2E Workflow Workers Coverage Report

Uh oh!

github-actions bot commented Feb 4, 2026

E2E Queue Consumers Coverage Report

Uh oh!

github-actions bot commented Feb 4, 2026

E2E SIMS API Coverage Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

dheepak-aot commented Jan 30, 2026 •

edited

Loading

sonarqubecloud bot commented Feb 4, 2026 •

edited

Loading