fix: DTOSS-11698 - Add GUID primary key to ServiceNow cases (#1761)

SamAinsworth-NHS · web-flow · commit ccb4764c077b · 2025-11-27T13:12:23.000Z
* feat(migrations): Add GUID primary key to ServiceNow cases

- Created a new migration to drop the existing primary key on SERVICENOW_CASES and add a new GUID primary key.
- Updated the DataServicesContextModelSnapshot to reflect the new ID property.
- Modified the ServicenowCase model to include the new GUID property.
- Updated unit tests to initialize the new ID property for ServicenowCase.
- Added Playwright tests to verify handling of duplicate ServiceNow case IDs.
- Updated environment configuration to reflect changes in unique key for ServiceNow cases.
- Created test payloads for duplicate ServiceNow case ID scenarios.

* rename: post-go-live-tests (files deleted, dirs gone) - changed to maintenance-regression-tests

* fix: attempt to diagnose disk issues during static analysis

* fix: second attempt to diagnose sonar analysis

* fix: removed problematic debugging task

* chore: remove pipeline debugging code (now in main)

* fix: regenerated migration for guid
diff --git a/application/CohortManager/src/Functions/ServiceNowIntegration/ServiceNowMessageHandler/ReceiveServiceNowMessageFunction.cs b/application/CohortManager/src/Functions/ServiceNowIntegration/ServiceNowMessageHandler/ReceiveServiceNowMessageFunction.cs
@@ -70,6 +70,7 @@ public async Task<HttpResponseData> Run(
 
             var serviceNowCase = new ServicenowCase
             {
+                Id = Guid.NewGuid(),
                 ServicenowId = requestBody.ServiceNowCaseNumber,
                 NhsNumber = nhsNumber,
                 Status = ServiceNowStatus.New,
diff --git a/application/CohortManager/src/Functions/Shared/DataServices.Database/DataServicesContext.cs b/application/CohortManager/src/Functions/Shared/DataServices.Database/DataServicesContext.cs
@@ -86,6 +86,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
             .ToTable("NEMS_SUBSCRIPTION", "dbo");
 
         modelBuilder.Entity<ServicenowCase>()
-            .ToTable("SERVICENOW_CASES", "dbo");
+            .ToTable("SERVICENOW_CASES", "dbo")
+            .HasIndex(s => s.ServicenowId, "IX_SERVICENOW_CASES_SERVICENOW_ID");
     }
 }
diff --git a/application/CohortManager/src/Functions/Shared/DataServices.Migrations/Migrations/20251125133634_AddGuidPrimaryKeyToServiceNowCases.Designer.cs b/application/CohortManager/src/Functions/Shared/DataServices.Migrations/Migrations/20251125133634_AddGuidPrimaryKeyToServiceNowCases.Designer.cs
diff --git a/application/CohortManager/src/Functions/Shared/DataServices.Migrations/Migrations/20251125133634_AddGuidPrimaryKeyToServiceNowCases.cs b/application/CohortManager/src/Functions/Shared/DataServices.Migrations/Migrations/20251125133634_AddGuidPrimaryKeyToServiceNowCases.cs
@@ -0,0 +1,65 @@
+﻿using System;
+using Microsoft.EntityFrameworkCore.Migrations;
+
+#nullable disable
+
+namespace DataServices.Migrations.Migrations
+{
+    /// <inheritdoc />
+    public partial class AddGuidPrimaryKeyToServiceNowCases : Migration
+    {
+        /// <inheritdoc />
+        protected override void Up(MigrationBuilder migrationBuilder)
+        {
+            migrationBuilder.DropPrimaryKey(
+                name: "PK_SERVICENOW_CASES",
+                schema: "dbo",
+                table: "SERVICENOW_CASES");
+
+            migrationBuilder.AddColumn<Guid>(
+                name: "ID",
+                schema: "dbo",
+                table: "SERVICENOW_CASES",
+                type: "uniqueidentifier",
+                nullable: false,
+                defaultValueSql: "NEWID()");
+
+            migrationBuilder.AddPrimaryKey(
+                name: "PK_SERVICENOW_CASES",
+                schema: "dbo",
+                table: "SERVICENOW_CASES",
+                column: "ID");
+
+            migrationBuilder.CreateIndex(
+                name: "IX_SERVICENOW_CASES_SERVICENOW_ID",
+                schema: "dbo",
+                table: "SERVICENOW_CASES",
+                column: "SERVICENOW_ID");
+        }
+
+        /// <inheritdoc />
+        protected override void Down(MigrationBuilder migrationBuilder)
+        {
+            migrationBuilder.DropPrimaryKey(
+                name: "PK_SERVICENOW_CASES",
+                schema: "dbo",
+                table: "SERVICENOW_CASES");
+
+            migrationBuilder.DropIndex(
+                name: "IX_SERVICENOW_CASES_SERVICENOW_ID",
+                schema: "dbo",
+                table: "SERVICENOW_CASES");
+
+            migrationBuilder.DropColumn(
+                name: "ID",
+                schema: "dbo",
+                table: "SERVICENOW_CASES");
+
+            migrationBuilder.AddPrimaryKey(
+                name: "PK_SERVICENOW_CASES",
+                schema: "dbo",
+                table: "SERVICENOW_CASES",
+                column: "SERVICENOW_ID");
+        }
+    }
+}
diff --git a/application/CohortManager/src/Functions/Shared/DataServices.Migrations/Migrations/DataServicesContextModelSnapshot.cs b/application/CohortManager/src/Functions/Shared/DataServices.Migrations/Migrations/DataServicesContextModelSnapshot.cs
@@ -1031,10 +1031,10 @@ protected override void BuildModel(ModelBuilder modelBuilder)
 
             modelBuilder.Entity("Model.ServicenowCase", b =>
                 {
-                    b.Property<string>("ServicenowId")
-                        .HasMaxLength(10)
-                        .HasColumnType("nvarchar(10)")
-                        .HasColumnName("SERVICENOW_ID");
+                    b.Property<Guid>("Id")
+                        .ValueGeneratedOnAdd()
+                        .HasColumnType("uniqueidentifier")
+                        .HasColumnName("ID");
 
                     b.Property<long?>("NhsNumber")
                         .HasColumnType("bigint")
@@ -1048,12 +1048,20 @@ protected override void BuildModel(ModelBuilder modelBuilder)
                         .HasColumnType("datetime")
                         .HasColumnName("RECORD_UPDATE_DATETIME");
 
+                    b.Property<string>("ServicenowId")
+                        .IsRequired()
+                        .HasMaxLength(10)
+                        .HasColumnType("nvarchar(10)")
+                        .HasColumnName("SERVICENOW_ID");
+
                     b.Property<string>("Status")
                         .HasMaxLength(10)
                         .HasColumnType("nvarchar(10)")
                         .HasColumnName("STATUS");
 
-                    b.HasKey("ServicenowId");
+                    b.HasKey("Id");
+
+                    b.HasIndex(new[] { "ServicenowId" }, "IX_SERVICENOW_CASES_SERVICENOW_ID");
 
                     b.ToTable("SERVICENOW_CASES", "dbo");
                 });
diff --git a/application/CohortManager/src/Functions/Shared/DataServices.Migrations/Program.cs b/application/CohortManager/src/Functions/Shared/DataServices.Migrations/Program.cs
@@ -53,7 +53,7 @@ static IHostBuilder CreateHostBuilder(DatabaseConfig config) =>
                     var sqlConnectionBuilder = new SqlConnectionStringBuilder(config.DtOsDatabaseConnectionString);
                     var connection = new SqlConnection(sqlConnectionBuilder.ConnectionString);
 
-                    if (config.SQL_IDENTITY_CLIENT_ID is not null)
+                    if (!string.IsNullOrWhiteSpace(config.SQL_IDENTITY_CLIENT_ID))
                     {
                         var credential = new ManagedIdentityCredential(config.SQL_IDENTITY_CLIENT_ID);
                         var token = credential.GetToken(new Azure.Core.TokenRequestContext(TokenScopes));
diff --git a/application/CohortManager/src/Functions/Shared/Model/EFModels/ServicenowCase.cs b/application/CohortManager/src/Functions/Shared/Model/EFModels/ServicenowCase.cs
@@ -6,6 +6,10 @@ namespace Model;
 public class ServicenowCase
 {
     [Key]
+    [Column("ID")]
+    public Guid Id { get; set; }
+
+    [Required]
     [MaxLength(10)]
     [Column("SERVICENOW_ID")]
     public required string ServicenowId { get; set; }
diff --git a/tests/UnitTests/ServiceNowCohortLookupTests/ServiceNowCohortLookupTests.cs b/tests/UnitTests/ServiceNowCohortLookupTests/ServiceNowCohortLookupTests.cs
@@ -50,6 +50,7 @@ public async Task Run_WithInvalidNhsNumber_LogsWarningAndSkips()
          // Arrange
          var invalidCase = new ServicenowCase
          {
+             Id = Guid.NewGuid(),
              ServicenowId = ServiceNowId,
              NhsNumber = 0, // Invalid NHS number
              Status = ServiceNowStatus.New,
@@ -84,6 +85,7 @@ public async Task Run_WithNoYesterdayParticipants_LogsInformation()
          var validNhsNumberLong = long.Parse(ValidNhsNumber);
          var newCase = new ServicenowCase
          {
+             Id = Guid.NewGuid(),
              ServicenowId = ServiceNowId,
              NhsNumber = validNhsNumberLong,
              Status = ServiceNowStatus.New,
@@ -118,6 +120,7 @@ public async Task Run_WithCaseAlreadyProcessed_DoesNotProcessAgain()
          var validNhsNumberLong = long.Parse(ValidNhsNumber);
          var processedCase = new ServicenowCase
          {
+             Id = Guid.NewGuid(),
              ServicenowId = ServiceNowId,
              NhsNumber = validNhsNumberLong,
              Status = ServiceNowStatus.Complete, // Already processed
@@ -143,20 +146,23 @@ public async Task Run_WithMixedCases_ProcessesCorrectly()
          // Arrange
          var validCase1 = new ServicenowCase
          {
+             Id = Guid.NewGuid(),
              ServicenowId = "SN1",
              NhsNumber = 123,
              Status = ServiceNowStatus.New
          };
 
          var validCase2 = new ServicenowCase
          {
+             Id = Guid.NewGuid(),
              ServicenowId = "SN2",
              NhsNumber = 456,
              Status = ServiceNowStatus.New
          };
 
          var invalidCase = new ServicenowCase
          {
+             Id = Guid.NewGuid(),
              ServicenowId = "SN3",
              NhsNumber = 0, // Invalid
              Status = ServiceNowStatus.New
@@ -220,13 +226,15 @@ public async Task Run_WithDuplicateNhsNumbers_ProcessesAll()
 
          var case1 = new ServicenowCase
          {
+             Id = Guid.NewGuid(),
              ServicenowId = "SN1",
              NhsNumber = testNhsNumber,
              Status = ServiceNowStatus.New
          };
 
          var case2 = new ServicenowCase
          {
+             Id = Guid.NewGuid(),
              ServicenowId = "SN2",
              NhsNumber = testNhsNumber, // Same NHS number
              Status = ServiceNowStatus.New
diff --git a/tests/playwright-tests/package.json b/tests/playwright-tests/package.json
@@ -16,6 +16,7 @@
     "test:regression_e2e_epic4d": "cross-env TEST_TYPE=RegressionEpic4d npx playwright test --project=dev --config=src/config/playwright.config.ts --grep=@epic4d-",
     "test:regression_e2e_epic4b": "cross-env TEST_TYPE=RegressionEpic4d npx playwright test --project=dev --config=src/config/playwright.config.ts --grep=@epic4b-",
     "test:regression_e2e_epic4f": "cross-env TEST_TYPE=RegressionEpic4f npx playwright test --project=dev --config=src/config/playwright.config.ts --grep=@epic4f-",
+    "test:maintenance_regression": "cross-env TEST_TYPE=Maintenance npx playwright test --project=dev --config=src/config/playwright.config.ts src/tests/e2e/maintenance-regression-tests/",
     "test": "cross-env TEST_TYPE=SMOKE npx playwright test --project=dev --config=src/config/playwright.config.ts --grep=\"@smoke @e2e\"",
     "test:validation-exceptions": "cross-env TEST_TYPE=VALIDATION npx playwright test tests/api/bsselect/validationExceptions/ --project=dev --config=src/config/playwright.config.ts",
     "test:service_now": "npx playwright test --project=dev --config=src/config/playwright.config.ts --grep=\"@service_now\"",
diff --git a/tests/playwright-tests/src/config/env.ts b/tests/playwright-tests/src/config/env.ts
@@ -82,6 +82,6 @@ export const config = {
   uniqueKeyParticipantManagement: 'ParticipantId',
   uniqueKeyParticipantDemographic: 'ParticipantId',
   uniqueKeyExceptionManagement: 'ExceptionId',
-  uniqueKeyServiceNowCases: 'ServicenowId',
+  uniqueKeyServiceNowCases: 'Id',
   ignoreValidationKey: 'apiEndpoint'
 }
diff --git a/tests/playwright-tests/src/tests/api/testFiles/@duplicate-servicenow-id/duplicate-servicenow-id-payload.json b/tests/playwright-tests/src/tests/api/testFiles/@duplicate-servicenow-id/duplicate-servicenow-id-payload.json
@@ -0,0 +1,26 @@
+{
+  "duplicateServiceNowId-first": {
+    "number": "CS9999999",
+    "u_case_variable_data": {
+      "nhs_number": "9990001111",
+      "forename_": "Test",
+      "surname_family_name": "Duplicate",
+      "date_of_birth": "1990-01-01",
+      "enter_dummy_gp_code": "ZZZDUP",
+      "BSO_code": "NORTH NOTTINGHAMSHIRE BREAST SCREENING OFFICE",
+      "reason_for_adding": "eligible_for_routine_screening_but_not_registered_with_a_gp_practice"
+    }
+  },
+  "duplicateServiceNowId-second": {
+    "number": "CS9999999",
+    "u_case_variable_data": {
+      "nhs_number": "9990001111",
+      "forename_": "Test",
+      "surname_family_name": "Duplicate",
+      "date_of_birth": "1990-01-01",
+      "enter_dummy_gp_code": "ZZZDUP",
+      "BSO_code": "NORTH NOTTINGHAMSHIRE BREAST SCREENING OFFICE",
+      "reason_for_adding": "eligible_for_routine_screening_but_not_registered_with_a_gp_practice"
+    }
+  }
+}
diff --git a/tests/playwright-tests/src/tests/e2e/maintenance-regression-tests/DTOSS-11543-duplicate-servicenow-id.spec.ts b/tests/playwright-tests/src/tests/e2e/maintenance-regression-tests/DTOSS-11543-duplicate-servicenow-id.spec.ts
@@ -0,0 +1,103 @@
+import { test, expect } from '@playwright/test';
+import { composeValidators, expectStatus } from '../../../api/responseValidators';
+import { receiveParticipantViaServiceNow } from '../../../api/distributionService/bsSelectService';
+import { loadParticipantPayloads } from '../../fixtures/jsonDataReader';
+import { ParticipantRecord } from '../../../interface/InputData';
+import { fetchApiResponse } from '../../../api/apiHelper';
+import { config } from '../../../config/env';
+
+// Maintenance regression test for DTOSS-11543 duplicate ServiceNow ID handling
+// Tags include maintenance-regression to allow selective execution
+
+test.describe.serial('@maintenance-regression @DTOSS-11543 @epic4c @api @duplicate-servicenow-id Test duplicate ServiceNow case ID handling', () => {
+  let participantData: Record<string, ParticipantRecord>;
+  const DUPLICATE_CASE_ID = 'CS9999999';
+  const SERVICE_NOW_DATA_SERVICE = config.serviceNowCasesDataService;
+
+  test.beforeAll(async () => {
+    const folderName = '@duplicate-servicenow-id';
+    const fileName = 'duplicate-servicenow-id-payload.json';
+    participantData = await loadParticipantPayloads(folderName, fileName);
+  });
+
+  test('@DTOSS-11543-1 First request with ServiceNow case ID should succeed', async ({ request }, testInfo) => {
+    testInfo.annotations.push({
+      type: 'Requirement - https://nhsd-jira.digital.nhs.uk/browse/DTOSS-11543',
+      description: 'Verifies that duplicate ServiceNow requests (10 seconds apart) do not cause exceptions',
+    });
+
+    const payload = participantData['duplicateServiceNowId-first'];
+
+    await test.step('Given a valid participant is received from ServiceNow, then response code is 202', async () => {
+      const response = await receiveParticipantViaServiceNow(request, payload);
+      const validators = composeValidators(
+        expectStatus(202)
+      );
+      await validators(response);
+    });
+  });
+
+  test('@DTOSS-11543-2 Second request with SAME ServiceNow case ID should also succeed (no primary key collision)', async ({ request }, testInfo) => {
+    testInfo.annotations.push({
+      type: 'Requirement - https://nhsd-jira.digital.nhs.uk/browse/DTOSS-11543',
+      description: 'Verifies that duplicate ServiceNow requests with same case ID do not cause primary key violations',
+    });
+
+    const payload = participantData['duplicateServiceNowId-second'];
+
+    await test.step('Given a duplicate ServiceNow case ID is received (simulating retry 10 seconds later), then response code is 202', async () => {
+      // Wait 10 seconds to simulate the actual scenario described in the bug
+      console.info('⏰ Waiting 10 seconds to simulate duplicate ServiceNow request...');
+      await new Promise(res => setTimeout(res, 10000));
+
+      const response = await receiveParticipantViaServiceNow(request, payload);
+      const validators = composeValidators(
+        expectStatus(202)
+      );
+      await validators(response);
+    });
+  });
+
+  test('@DTOSS-11543-3 Verify both records exist in database with different GUIDs', async ({ request }, testInfo) => {
+    testInfo.annotations.push({
+      type: 'Requirement - https://nhsd-jira.digital.nhs.uk/browse/DTOSS-11543',
+      description: 'Verifies that both records with the same ServiceNow case ID exist in database with unique GUIDs',
+    });
+
+    await test.step('Query ServiceNowCasesDataService to get all records with the duplicate case ID', async () => {
+      // Wait a bit for the data to be persisted
+      await new Promise(res => setTimeout(res, 2000));
+
+      const response = await fetchApiResponse(`api/${SERVICE_NOW_DATA_SERVICE}`, request);
+      expect(response.ok()).toBeTruthy();
+
+      const allRecords = await response.json();
+      expect(Array.isArray(allRecords)).toBeTruthy();
+
+      // Filter records by ServicenowId
+      const duplicateRecords = allRecords.filter((record: any) => record.ServicenowId === DUPLICATE_CASE_ID);
+
+      console.info(`Found ${duplicateRecords.length} records with ServiceNow case ID ${DUPLICATE_CASE_ID}`);
+      console.info('Records:', JSON.stringify(duplicateRecords, null, 2));
+
+      // Verify we have at least 2 records with the same ServicenowId
+      expect(duplicateRecords.length).toBeGreaterThanOrEqual(2);
+
+      // Extract the ID (GUID) values
+      const ids = duplicateRecords.map((record: any) => record.Id);
+
+      // Verify all IDs are unique (no duplicates in the array)
+      const uniqueIds = new Set(ids);
+      expect(uniqueIds.size).toBe(ids.length);
+
+      // Verify all IDs are valid GUIDs (format check)
+      const guidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
+      for (const id of ids) {
+        expect(id).toMatch(guidRegex);
+      }
+
+      console.info(`✅ Verified ${duplicateRecords.length} records with same ServiceNowId but unique GUIDs`);
+      console.info('Unique IDs:', ids);
+    });
+  });
+});

Original file line number	Diff line number	Diff line change
`@@ -70,6 +70,7 @@ public async Task<HttpResponseData> Run(`
`70`	`70`
`71`	`71`	`var serviceNowCase = new ServicenowCase`
`72`	`72`	`{`
	`73`	`+ Id = Guid.NewGuid(),`
`73`	`74`	`ServicenowId = requestBody.ServiceNowCaseNumber,`
`74`	`75`	`NhsNumber = nhsNumber,`
`75`	`76`	`Status = ServiceNowStatus.New,`
Original file line number	Diff line number	Diff line change
`@@ -86,6 +86,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)`
`86`	`86`	`.ToTable("NEMS_SUBSCRIPTION", "dbo");`
`87`	`87`
`88`	`88`	`modelBuilder.Entity<ServicenowCase>()`
`89`		`- .ToTable("SERVICENOW_CASES", "dbo");`
	`89`	`+ .ToTable("SERVICENOW_CASES", "dbo")`
	`90`	`+ .HasIndex(s => s.ServicenowId, "IX_SERVICENOW_CASES_SERVICENOW_ID");`
`90`	`91`	`}`
`91`	`92`	`}`
Original file line number	Diff line number	Diff line change
`@@ -53,7 +53,7 @@ static IHostBuilder CreateHostBuilder(DatabaseConfig config) =>`
`53`	`53`	`var sqlConnectionBuilder = new SqlConnectionStringBuilder(config.DtOsDatabaseConnectionString);`
`54`	`54`	`var connection = new SqlConnection(sqlConnectionBuilder.ConnectionString);`
`55`	`55`
`56`		`- if (config.SQL_IDENTITY_CLIENT_ID is not null)`
	`56`	`+ if (!string.IsNullOrWhiteSpace(config.SQL_IDENTITY_CLIENT_ID))`
`57`	`57`	`{`
`58`	`58`	`var credential = new ManagedIdentityCredential(config.SQL_IDENTITY_CLIENT_ID);`
`59`	`59`	`var token = credential.GetToken(new Azure.Core.TokenRequestContext(TokenScopes));`
Original file line number	Diff line number	Diff line change
`@@ -82,6 +82,6 @@ export const config = {`
`82`	`82`	`uniqueKeyParticipantManagement: 'ParticipantId',`
`83`	`83`	`uniqueKeyParticipantDemographic: 'ParticipantId',`
`84`	`84`	`uniqueKeyExceptionManagement: 'ExceptionId',`
`85`		`- uniqueKeyServiceNowCases: 'ServicenowId',`
	`85`	`+ uniqueKeyServiceNowCases: 'Id',`
`86`	`86`	`ignoreValidationKey: 'apiEndpoint'`
`87`	`87`	`}`