From 71a808c7448274b25ff5e39e78480d0d7b5ea467 Mon Sep 17 00:00:00 2001 From: Tom Hawkin Date: Tue, 14 Jul 2026 10:12:47 +0100 Subject: [PATCH 1/4] added UpdatePerson mutation to save strong match NHS numbers back to eclipse. refactored graphQLProcessor --- .../Infrastructure/GraphQLProcessor.cs | 176 ++++++++++++------ .../PersonByCriteria.graphql | 1 + .../UpdatePerson.graphql | 7 + .../GraphQlProcessorTests.cs | 92 +++++++++ .../FakeEclipseGraphQLApi/Models/Person.cs | 1 + .../Models/UpdatePerson.cs | 17 ++ tests/tools/FakeEclipseGraphQLApi/Mutation.cs | 51 +++++ tests/tools/FakeEclipseGraphQLApi/Program.cs | 1 + 8 files changed, 289 insertions(+), 57 deletions(-) create mode 100644 src/SUI.Client/SUI.Client.GraphQLProcessJob/UpdatePerson.graphql create mode 100644 tests/tools/FakeEclipseGraphQLApi/Models/UpdatePerson.cs create mode 100644 tests/tools/FakeEclipseGraphQLApi/Mutation.cs diff --git a/src/SUI.Client/SUI.Client.GraphQLProcessJob/Infrastructure/GraphQLProcessor.cs b/src/SUI.Client/SUI.Client.GraphQLProcessJob/Infrastructure/GraphQLProcessor.cs index 74a248df..b542206a 100644 --- a/src/SUI.Client/SUI.Client.GraphQLProcessJob/Infrastructure/GraphQLProcessor.cs +++ b/src/SUI.Client/SUI.Client.GraphQLProcessJob/Infrastructure/GraphQLProcessor.cs @@ -7,6 +7,7 @@ using SUI.Client.Core.Application.Interfaces; using SUI.Client.Core.Application.Models; +using SUI.Client.Core.Application.UseCases.MatchPeople; using SUI.Client.Core.Infrastructure.CsvParsers; namespace SUI.Client.GraphQLProcessJob.Infrastructure; @@ -21,83 +22,144 @@ public class GraphQlProcessor( public async Task RunAsync(CancellationToken cancellationToken) { logger.LogInformation("Running Graph QL Process Job."); + var mappings = csvMatchDataOptions.Value.ColumnMappings; + + var (csvRecords, personObjectVersions) = await FetchAndCompilePersonRecordsAsync(mappings, cancellationToken); + + logger.LogInformation("Completed compiling GraphQL records. Total records retrieved: {Count}.", + csvRecords.Count); + + var matchedResults = await matchPersonRecordOrchestrator.ProcessAsync( + csvRecords, + "graphql_extract", + cancellationToken + ); + + logger.LogInformation( + "Finished processing matching with orchestrator. Result count: {Count}. Matches: {MatchCount}", + matchedResults.Count, matchedResults.Count(x => x.ApiResult is + { + Result.IsHighConfidenceMatch: true + })); + + await SaveMatchedNhsNumbersAsync(matchedResults, personObjectVersions, mappings, cancellationToken); + } + + private async Task<(List CsvRecords, Dictionary PersonObjectVersions)> FetchAndCompilePersonRecordsAsync( + CsvMatchDataOptions.Headers mappings, + CancellationToken cancellationToken) + { int pageNumber = 1; const int pageSize = 10; - bool hasMoreResults = true; var csvRecords = new List(); - var mappings = csvMatchDataOptions.Value.ColumnMappings; + var personObjectVersions = new Dictionary(); - while (hasMoreResults && !cancellationToken.IsCancellationRequested) + while (!cancellationToken.IsCancellationRequested) { var results = await eclipseClient.PersonByCriteria.ExecuteAsync(options.Value.MaxAge, new RequestCursorInput { PageNumber = pageNumber, PageSize = pageSize }, cancellationToken); results.EnsureNoErrors(); - if (results.Data?.PersonByCriteria?.Results is { Count: > 0 } resultsList) + var resultsList = results.Data?.PersonByCriteria?.Results; + if (resultsList == null || resultsList.Count == 0) { - foreach (var result in resultsList) - { - if (result is not IPersonByCriteria_PersonByCriteria_Results_Person person) - { - continue; - } - - var personDictionary = new Dictionary - { - { mappings.Id, person.Id }, - { mappings.Given, person.Forename ?? "" }, - { mappings.Family, person.Surname ?? "" }, - { mappings.BirthDate, person.DateOfBirth?.Lower?.ToString(csvMatchDataOptions.Value.DateFormat) ?? "" }, - { - mappings.Postcode, - person.Addresses.FirstOrDefault(a => a.Id == person.PreferredAddress?.Id)?.Location - ?.Postcode ?? "" - } - }; - - if (!string.IsNullOrEmpty(mappings.NhsNumber)) - { - personDictionary[mappings.NhsNumber] = person.NhsNumber ?? ""; - } - - if (!string.IsNullOrEmpty(mappings.Gender)) - { - personDictionary[mappings.Gender] = person.Gender?.ToString().ToLower() ?? ""; - } - - csvRecords.Add(new CsvRecordDto(personDictionary)); - } + break; + } - var cursor = results.Data?.PersonByCriteria?.Cursor; - if (cursor != null && cursor.Offset + cursor.Returned < cursor.TotalSize) - { - pageNumber++; - } - else + foreach (var result in resultsList) + { + if (result is IPersonByCriteria_PersonByCriteria_Results_Person person) { - hasMoreResults = false; + personObjectVersions[person.Id] = person.ObjectVersion; + csvRecords.Add(new CsvRecordDto(MapPersonToDictionary(person, mappings))); } } - else + + var cursor = results.Data?.PersonByCriteria?.Cursor; + if (cursor == null || cursor.Offset + cursor.Returned >= cursor.TotalSize) { - hasMoreResults = false; + break; } + + pageNumber++; } - logger.LogInformation("Completed compiling GraphQL records. Total records retrieved: {Count}.", - csvRecords.Count); + return (csvRecords, personObjectVersions); + } - var matchedResults = await matchPersonRecordOrchestrator.ProcessAsync( - csvRecords, - "graphql_extract", - cancellationToken - ); + private Dictionary MapPersonToDictionary( + IPersonByCriteria_PersonByCriteria_Results_Person person, + CsvMatchDataOptions.Headers mappings) + { + var personDictionary = new Dictionary + { + { mappings.Id, person.Id }, + { mappings.Given, person.Forename ?? "" }, + { mappings.Family, person.Surname ?? "" }, + { mappings.BirthDate, person.DateOfBirth?.Lower?.ToString(csvMatchDataOptions.Value.DateFormat) ?? "" }, + { mappings.Postcode, GetPreferredPostcode(person) } + }; - logger.LogInformation( - "Finished processing matching with orchestrator. Result count: {Count}. Matches: {MatchCount}", - matchedResults.Count, matchedResults.Count(x => x.ApiResult is + if (!string.IsNullOrEmpty(mappings.NhsNumber)) + { + personDictionary[mappings.NhsNumber] = person.NhsNumber ?? ""; + } + + if (!string.IsNullOrEmpty(mappings.Gender)) + { + personDictionary[mappings.Gender] = person.Gender?.ToString().ToLower() ?? ""; + } + + return personDictionary; + } + + private static string GetPreferredPostcode(IPersonByCriteria_PersonByCriteria_Results_Person person) => + person.Addresses.FirstOrDefault(a => a.Id == person.PreferredAddress?.Id)?.Location?.Postcode ?? ""; + + private async Task SaveMatchedNhsNumbersAsync( + IEnumerable> matchedResults, + Dictionary personObjectVersions, + CsvMatchDataOptions.Headers mappings, + CancellationToken cancellationToken) + { + foreach (var result in matchedResults) + { + if (result.ApiResult is not { Result.IsHighConfidenceMatch: true } || + string.IsNullOrEmpty(result.ApiResult.Result.NhsNumber)) { - Result.IsHighConfidenceMatch: true - })); + continue; + } + + var personId = result.OriginalData.Record[mappings.Id]; + var matchedNhsNumber = result.ApiResult.Result.NhsNumber; + + if (!personObjectVersions.TryGetValue(personId, out var objectVersion)) + { + logger.LogWarning("Could not find ObjectVersion for Person {PersonId}. Skipping NHS number update.", personId); + continue; + } + + logger.LogInformation("Saving matched NHS number {NhsNumber} for Person {PersonId} with ObjectVersion {ObjectVersion}.", + matchedNhsNumber, personId, objectVersion); + + try + { + var updateInput = new UpdatePerson + { + Id = personId, + NhsNumber = matchedNhsNumber, + ObjectVersion = objectVersion + }; + + var updateResult = await eclipseClient.UpdatePerson.ExecuteAsync(updateInput, cancellationToken); + updateResult.EnsureNoErrors(); + + logger.LogInformation("Successfully saved NHS number for Person {PersonId}.", personId); + } + catch (Exception ex) + { + logger.LogError(ex, "Failed to save NHS number for Person {PersonId}.", personId); + } + } } } \ No newline at end of file diff --git a/src/SUI.Client/SUI.Client.GraphQLProcessJob/PersonByCriteria.graphql b/src/SUI.Client/SUI.Client.GraphQLProcessJob/PersonByCriteria.graphql index 69e3acfe..4f55f115 100644 --- a/src/SUI.Client/SUI.Client.GraphQLProcessJob/PersonByCriteria.graphql +++ b/src/SUI.Client/SUI.Client.GraphQLProcessJob/PersonByCriteria.graphql @@ -22,6 +22,7 @@ query PersonByCriteria($maxAge: Int, $paging: RequestCursorInput) { mask } id + objectVersion forename surname gender diff --git a/src/SUI.Client/SUI.Client.GraphQLProcessJob/UpdatePerson.graphql b/src/SUI.Client/SUI.Client.GraphQLProcessJob/UpdatePerson.graphql new file mode 100644 index 00000000..80b489e9 --- /dev/null +++ b/src/SUI.Client/SUI.Client.GraphQLProcessJob/UpdatePerson.graphql @@ -0,0 +1,7 @@ +mutation UpdatePerson($input: UpdatePerson!) { + updatePerson(input: $input) { + id + objectVersion + nhsNumber + } +} diff --git a/tests/Unit.Tests/Client/GraphQlProcessJob/GraphQlProcessorTests.cs b/tests/Unit.Tests/Client/GraphQlProcessJob/GraphQlProcessorTests.cs index 3715bf56..50aecfdd 100644 --- a/tests/Unit.Tests/Client/GraphQlProcessJob/GraphQlProcessorTests.cs +++ b/tests/Unit.Tests/Client/GraphQlProcessJob/GraphQlProcessorTests.cs @@ -852,4 +852,96 @@ public async Task RunAsync_ShouldNotMapNhsNumberAndGender_WhenMissingFromConfig( Assert.False(record.Record.ContainsKey("NHSNumber")); Assert.False(record.Record.ContainsKey("Gender")); } + + [Fact] + public async Task RunAsync_ShouldCallUpdatePersonMutation_WhenHighConfidenceMatchIsReturned() + { + // Arrange + var options = Options.Create(new GraphQlProcessJobOptions { MaxAge = 25 }); + + var updatePersonResultMock = new Mock(); + var operationUpdateResultMock = new Mock>(); + operationUpdateResultMock.Setup(r => r.Errors).Returns(new List()); + operationUpdateResultMock.Setup(r => r.Data).Returns(updatePersonResultMock.Object); + + var updatePersonMutationMock = new Mock(); + updatePersonMutationMock + .Setup(m => m.ExecuteAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync(operationUpdateResultMock.Object); + + _eclipseClientMock.Setup(c => c.UpdatePerson).Returns(updatePersonMutationMock.Object); + + var sut = new GraphQlProcessor( + _loggerMock.Object, + _eclipseClientMock.Object, + _matchPersonRecordOrchestratorMock.Object, + options, + _csvMatchOptions + ); + + var personMock = new Mock(); + personMock.Setup(p => p.Id).Returns("person-123"); + personMock.Setup(p => p.ObjectVersion).Returns(5); + personMock.Setup(p => p.Forename).Returns("John"); + personMock.Setup(p => p.Surname).Returns("Doe"); + personMock.Setup(p => p.DateOfBirth).Returns((IPersonByCriteria_PersonByCriteria_Results_DateOfBirth?)null); + personMock.Setup(p => p.Addresses).Returns(new List()); + personMock.Setup(p => p.PreferredAddress) + .Returns((IPersonByCriteria_PersonByCriteria_Results_PreferredAddress?)null); + + var resultsList = new List { personMock.Object }; + + var cursorMock = new Mock(); + cursorMock.Setup(c => c.Offset).Returns(0); + cursorMock.Setup(c => c.Returned).Returns(1); + cursorMock.Setup(c => c.TotalSize).Returns(1); + + var personByCriteriaMock = new Mock(); + personByCriteriaMock.Setup(p => p.Results).Returns(resultsList.AsReadOnly()); + personByCriteriaMock.Setup(p => p.Cursor).Returns(cursorMock.Object); + + var operationResultDataMock = new Mock(); + operationResultDataMock.Setup(o => o.PersonByCriteria).Returns(personByCriteriaMock.Object); + + var operationResultMock = new Mock>(); + operationResultMock.Setup(r => r.Data).Returns(operationResultDataMock.Object); + operationResultMock.Setup(r => r.Errors).Returns(new List()); + + _personByCriteriaQueryMock + .Setup(q => q.ExecuteAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(operationResultMock.Object); + + var matchResult = new Shared.Models.MatchResult + { + MatchStatus = Shared.Models.MatchStatus.Match, + NhsNumber = "9999999999", + Score = 1.0m + }; + var apiResult = new Shared.Models.PersonMatchResponse { Result = matchResult }; + + var matchedRecord = new ProcessedMatchRecord + { + OriginalData = new CsvRecordDto(new Dictionary { { "SourceID", "person-123" } }), + ApiResult = apiResult, + IsSuccess = true + }; + + _matchPersonRecordOrchestratorMock + .Setup(o => o.ProcessAsync(It.IsAny>(), It.IsAny(), + It.IsAny())) + .ReturnsAsync(new List> { matchedRecord }); + + // Act + await sut.RunAsync(CancellationToken.None); + + // Assert + updatePersonMutationMock.Verify( + m => m.ExecuteAsync( + It.Is(input => + input.Id == "person-123" && + input.NhsNumber == "9999999999" && + input.ObjectVersion == 5), + It.IsAny()), + Times.Once); + } } \ No newline at end of file diff --git a/tests/tools/FakeEclipseGraphQLApi/Models/Person.cs b/tests/tools/FakeEclipseGraphQLApi/Models/Person.cs index e5f48ae8..ef0296de 100644 --- a/tests/tools/FakeEclipseGraphQLApi/Models/Person.cs +++ b/tests/tools/FakeEclipseGraphQLApi/Models/Person.cs @@ -11,6 +11,7 @@ public class Person : IPersonByCriteria_PersonByCriteria_Results public string? Surname { get; set; } public string? Gender { get; set; } public string? NhsNumber { get; set; } + public int ObjectVersion { get; set; } = 1; public DateRange? DateOfBirth { get; set; } public List
Addresses { get; set; } = new(); public Address? PreferredAddress { get; set; } diff --git a/tests/tools/FakeEclipseGraphQLApi/Models/UpdatePerson.cs b/tests/tools/FakeEclipseGraphQLApi/Models/UpdatePerson.cs new file mode 100644 index 00000000..d822bb2c --- /dev/null +++ b/tests/tools/FakeEclipseGraphQLApi/Models/UpdatePerson.cs @@ -0,0 +1,17 @@ +using System.Diagnostics.CodeAnalysis; + +using HotChocolate; + +namespace FakeEclipseGraphQLApi.Models; + +[ExcludeFromCodeCoverage] +[InputObjectType(Name = "UpdatePerson")] +public class UpdatePerson +{ + [GraphQLType(typeof(NonNullType))] + public string Id { get; set; } = string.Empty; + + public string? NhsNumber { get; set; } + + public int ObjectVersion { get; set; } +} \ No newline at end of file diff --git a/tests/tools/FakeEclipseGraphQLApi/Mutation.cs b/tests/tools/FakeEclipseGraphQLApi/Mutation.cs new file mode 100644 index 00000000..0aea72c8 --- /dev/null +++ b/tests/tools/FakeEclipseGraphQLApi/Mutation.cs @@ -0,0 +1,51 @@ +using System.Diagnostics.CodeAnalysis; +using System.Text.Json; + +using FakeEclipseGraphQLApi.Models; + +namespace FakeEclipseGraphQLApi; + +[ExcludeFromCodeCoverage] +public class Mutation +{ + public Person UpdatePerson(UpdatePerson input) + { + var filePath = System.IO.Path.Combine(AppContext.BaseDirectory, "data", "personByCriteria.json"); + if (!File.Exists(filePath)) + { + filePath = System.IO.Path.Combine("data", "personByCriteria.json"); + } + + if (!File.Exists(filePath)) + { + throw new FileNotFoundException($"Mock data file not found at: {filePath}"); + } + + var json = File.ReadAllText(filePath); + var options = new JsonSerializerOptions + { + PropertyNameCaseInsensitive = true, + WriteIndented = true + }; + var response = JsonSerializer.Deserialize(json, options); + + if (response == null || response.Results == null) + { + throw new InvalidOperationException("Failed to load and deserialize person data."); + } + + var person = response.Results.FirstOrDefault(p => p.Id == input.Id); + if (person == null) + { + throw new ArgumentException($"Person with ID '{input.Id}' not found."); + } + + person.NhsNumber = input.NhsNumber; + + var updatedJson = JsonSerializer.Serialize(response, options); + + File.WriteAllText(filePath, updatedJson); + + return person; + } +} \ No newline at end of file diff --git a/tests/tools/FakeEclipseGraphQLApi/Program.cs b/tests/tools/FakeEclipseGraphQLApi/Program.cs index d1611d99..2dd57caa 100644 --- a/tests/tools/FakeEclipseGraphQLApi/Program.cs +++ b/tests/tools/FakeEclipseGraphQLApi/Program.cs @@ -7,6 +7,7 @@ builder.Services .AddGraphQLServer() .AddQueryType() + .AddMutationType() .AddUnionType() .AddType(); From 1f2cb9b4bea3601013783a143b21768f90cbcbac Mon Sep 17 00:00:00 2001 From: Tom Hawkin Date: Wed, 15 Jul 2026 12:10:28 +0100 Subject: [PATCH 2/4] changed paging to 100 items at a time. added more logging. added a job timer. fixed an issue with mutations not passing validation because they were missing the personTypes property --- .../Infrastructure/GraphQLProcessor.cs | 59 ++++++++---- .../PersonByCriteria.graphql | 1 + .../appsettings.json | 3 +- .../GraphQlProcessorTests.cs | 96 ++++++++++++++++++- .../FakeEclipseGraphQLApi/Models/Person.cs | 1 + .../Models/PersonType.cs | 10 ++ .../Models/UpdatePerson.cs | 2 + 7 files changed, 154 insertions(+), 18 deletions(-) create mode 100644 tests/tools/FakeEclipseGraphQLApi/Models/PersonType.cs diff --git a/src/SUI.Client/SUI.Client.GraphQLProcessJob/Infrastructure/GraphQLProcessor.cs b/src/SUI.Client/SUI.Client.GraphQLProcessJob/Infrastructure/GraphQLProcessor.cs index b542206a..bad1d82f 100644 --- a/src/SUI.Client/SUI.Client.GraphQLProcessJob/Infrastructure/GraphQLProcessor.cs +++ b/src/SUI.Client/SUI.Client.GraphQLProcessJob/Infrastructure/GraphQLProcessor.cs @@ -1,3 +1,5 @@ +using System.Diagnostics; + using Eclipse.GraphQL; using Microsoft.Extensions.Logging; @@ -19,15 +21,18 @@ public class GraphQlProcessor( IOptions options, IOptions csvMatchDataOptions) { + private record PersonMetadata(int ObjectVersion, string? NhsNumber, IReadOnlyList PersonTypes); + public async Task RunAsync(CancellationToken cancellationToken) { + var timer = Stopwatch.StartNew(); logger.LogInformation("Running Graph QL Process Job."); var mappings = csvMatchDataOptions.Value.ColumnMappings; - var (csvRecords, personObjectVersions) = await FetchAndCompilePersonRecordsAsync(mappings, cancellationToken); + (List csvRecords, Dictionary personMetadata) = await FetchAndCompilePersonRecordsAsync(mappings, cancellationToken); - logger.LogInformation("Completed compiling GraphQL records. Total records retrieved: {Count}.", - csvRecords.Count); + logger.LogInformation("Completed compiling GraphQL records. Total records retrieved: {Count}. Elapsed Time: {ElapsedTime}", + csvRecords.Count, timer.Elapsed.ToString("g")); var matchedResults = await matchPersonRecordOrchestrator.ProcessAsync( csvRecords, @@ -36,23 +41,25 @@ public async Task RunAsync(CancellationToken cancellationToken) ); logger.LogInformation( - "Finished processing matching with orchestrator. Result count: {Count}. Matches: {MatchCount}", + "Finished processing matching. Result count: {Count}. Matches: {MatchCount}. Elapsed time: {ElapsedTime}", matchedResults.Count, matchedResults.Count(x => x.ApiResult is { Result.IsHighConfidenceMatch: true - })); + }), timer.Elapsed.ToString("g")); + + await SaveMatchedNhsNumbersAsync(matchedResults, personMetadata, mappings, cancellationToken); - await SaveMatchedNhsNumbersAsync(matchedResults, personObjectVersions, mappings, cancellationToken); + logger.LogInformation("GraphQL Job Complete. Total time: {ElapsedTime}", timer.Elapsed.ToString("g")); } - private async Task<(List CsvRecords, Dictionary PersonObjectVersions)> FetchAndCompilePersonRecordsAsync( + private async Task<(List CsvRecords, Dictionary PersonMetadata)> FetchAndCompilePersonRecordsAsync( CsvMatchDataOptions.Headers mappings, CancellationToken cancellationToken) { int pageNumber = 1; - const int pageSize = 10; + const int pageSize = 100; var csvRecords = new List(); - var personObjectVersions = new Dictionary(); + var personMetadata = new Dictionary(); while (!cancellationToken.IsCancellationRequested) { @@ -70,12 +77,23 @@ public async Task RunAsync(CancellationToken cancellationToken) { if (result is IPersonByCriteria_PersonByCriteria_Results_Person person) { - personObjectVersions[person.Id] = person.ObjectVersion; + personMetadata[person.Id] = new PersonMetadata( + person.ObjectVersion, + person.NhsNumber, + person.PersonTypes ?? [] + ); csvRecords.Add(new CsvRecordDto(MapPersonToDictionary(person, mappings))); } } var cursor = results.Data?.PersonByCriteria?.Cursor; + if (cursor != null && logger.IsEnabled(LogLevel.Information)) + { + logger.LogInformation( + "Processed Page: {Page}. Page size: {PageSize}. Total Records: {TotalRecords}", + cursor.PageNumber, cursor.PageSize, cursor.TotalSize); + } + if (cursor == null || cursor.Offset + cursor.Returned >= cursor.TotalSize) { break; @@ -84,7 +102,7 @@ public async Task RunAsync(CancellationToken cancellationToken) pageNumber++; } - return (csvRecords, personObjectVersions); + return (csvRecords, personMetadata); } private Dictionary MapPersonToDictionary( @@ -118,7 +136,7 @@ private static string GetPreferredPostcode(IPersonByCriteria_PersonByCriteria_Re private async Task SaveMatchedNhsNumbersAsync( IEnumerable> matchedResults, - Dictionary personObjectVersions, + Dictionary personMetadata, CsvMatchDataOptions.Headers mappings, CancellationToken cancellationToken) { @@ -127,20 +145,28 @@ private async Task SaveMatchedNhsNumbersAsync( if (result.ApiResult is not { Result.IsHighConfidenceMatch: true } || string.IsNullOrEmpty(result.ApiResult.Result.NhsNumber)) { + logger.LogInformation("Match is low confidence, Skipping update."); continue; } var personId = result.OriginalData.Record[mappings.Id]; var matchedNhsNumber = result.ApiResult.Result.NhsNumber; - if (!personObjectVersions.TryGetValue(personId, out var objectVersion)) + if (!personMetadata.TryGetValue(personId, out var metadata)) + { + logger.LogWarning("Could not find metadata for Person {PersonId}. Skipping NHS number update.", personId); + continue; + } + + if (!string.IsNullOrEmpty(metadata.NhsNumber)) { - logger.LogWarning("Could not find ObjectVersion for Person {PersonId}. Skipping NHS number update.", personId); + logger.LogInformation("Person {PersonId} already has NHS number {ExistingNhsNumber}. Skipping update.", + personId, metadata.NhsNumber); continue; } logger.LogInformation("Saving matched NHS number {NhsNumber} for Person {PersonId} with ObjectVersion {ObjectVersion}.", - matchedNhsNumber, personId, objectVersion); + matchedNhsNumber, personId, metadata.ObjectVersion); try { @@ -148,7 +174,8 @@ private async Task SaveMatchedNhsNumbersAsync( { Id = personId, NhsNumber = matchedNhsNumber, - ObjectVersion = objectVersion + ObjectVersion = metadata.ObjectVersion, + PersonTypes = metadata.PersonTypes }; var updateResult = await eclipseClient.UpdatePerson.ExecuteAsync(updateInput, cancellationToken); diff --git a/src/SUI.Client/SUI.Client.GraphQLProcessJob/PersonByCriteria.graphql b/src/SUI.Client/SUI.Client.GraphQLProcessJob/PersonByCriteria.graphql index 4f55f115..c89b4517 100644 --- a/src/SUI.Client/SUI.Client.GraphQLProcessJob/PersonByCriteria.graphql +++ b/src/SUI.Client/SUI.Client.GraphQLProcessJob/PersonByCriteria.graphql @@ -23,6 +23,7 @@ query PersonByCriteria($maxAge: Int, $paging: RequestCursorInput) { } id objectVersion + personTypes forename surname gender diff --git a/src/SUI.Client/SUI.Client.GraphQLProcessJob/appsettings.json b/src/SUI.Client/SUI.Client.GraphQLProcessJob/appsettings.json index 45dab8eb..9aa0cc1e 100644 --- a/src/SUI.Client/SUI.Client.GraphQLProcessJob/appsettings.json +++ b/src/SUI.Client/SUI.Client.GraphQLProcessJob/appsettings.json @@ -2,7 +2,8 @@ "Logging": { "LogLevel": { "Default": "Information", - "Microsoft.Hosting.Lifetime": "Information" + "Microsoft.Hosting.Lifetime": "Information", + "System.Net.Http.HttpClient": "Error" } }, "GraphQlProcessJob": { diff --git a/tests/Unit.Tests/Client/GraphQlProcessJob/GraphQlProcessorTests.cs b/tests/Unit.Tests/Client/GraphQlProcessJob/GraphQlProcessorTests.cs index 50aecfdd..3ed6f52f 100644 --- a/tests/Unit.Tests/Client/GraphQlProcessJob/GraphQlProcessorTests.cs +++ b/tests/Unit.Tests/Client/GraphQlProcessJob/GraphQlProcessorTests.cs @@ -882,6 +882,7 @@ public async Task RunAsync_ShouldCallUpdatePersonMutation_WhenHighConfidenceMatc var personMock = new Mock(); personMock.Setup(p => p.Id).Returns("person-123"); personMock.Setup(p => p.ObjectVersion).Returns(5); + personMock.Setup(p => p.PersonTypes).Returns(new List { PersonType.Client }); personMock.Setup(p => p.Forename).Returns("John"); personMock.Setup(p => p.Surname).Returns("Doe"); personMock.Setup(p => p.DateOfBirth).Returns((IPersonByCriteria_PersonByCriteria_Results_DateOfBirth?)null); @@ -940,8 +941,101 @@ public async Task RunAsync_ShouldCallUpdatePersonMutation_WhenHighConfidenceMatc It.Is(input => input.Id == "person-123" && input.NhsNumber == "9999999999" && - input.ObjectVersion == 5), + input.ObjectVersion == 5 && + input.PersonTypes != null && + input.PersonTypes.Count == 1 && + input.PersonTypes[0] == PersonType.Client), It.IsAny()), Times.Once); } + + [Fact] + public async Task RunAsync_ShouldNotCallUpdatePersonMutation_WhenPersonAlreadyHasNhsNumber() + { + // Arrange + var options = Options.Create(new GraphQlProcessJobOptions { MaxAge = 25 }); + + var updatePersonResultMock = new Mock(); + var operationUpdateResultMock = new Mock>(); + operationUpdateResultMock.Setup(r => r.Errors).Returns(new List()); + operationUpdateResultMock.Setup(r => r.Data).Returns(updatePersonResultMock.Object); + + var updatePersonMutationMock = new Mock(); + updatePersonMutationMock + .Setup(m => m.ExecuteAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync(operationUpdateResultMock.Object); + + _eclipseClientMock.Setup(c => c.UpdatePerson).Returns(updatePersonMutationMock.Object); + + var sut = new GraphQlProcessor( + _loggerMock.Object, + _eclipseClientMock.Object, + _matchPersonRecordOrchestratorMock.Object, + options, + _csvMatchOptions + ); + + // Person already has NHS number "1111111111" + var personMock = new Mock(); + personMock.Setup(p => p.Id).Returns("person-123"); + personMock.Setup(p => p.NhsNumber).Returns("1111111111"); + personMock.Setup(p => p.ObjectVersion).Returns(5); + personMock.Setup(p => p.Forename).Returns("John"); + personMock.Setup(p => p.Surname).Returns("Doe"); + personMock.Setup(p => p.DateOfBirth).Returns((IPersonByCriteria_PersonByCriteria_Results_DateOfBirth?)null); + personMock.Setup(p => p.Addresses).Returns(new List()); + personMock.Setup(p => p.PreferredAddress).Returns((IPersonByCriteria_PersonByCriteria_Results_PreferredAddress?)null); + + var resultsList = new List { personMock.Object }; + + var cursorMock = new Mock(); + cursorMock.Setup(c => c.Offset).Returns(0); + cursorMock.Setup(c => c.Returned).Returns(1); + cursorMock.Setup(c => c.TotalSize).Returns(1); + + var personByCriteriaMock = new Mock(); + personByCriteriaMock.Setup(p => p.Results).Returns(resultsList.AsReadOnly()); + personByCriteriaMock.Setup(p => p.Cursor).Returns(cursorMock.Object); + + var operationResultDataMock = new Mock(); + operationResultDataMock.Setup(o => o.PersonByCriteria).Returns(personByCriteriaMock.Object); + + var operationResultMock = new Mock>(); + operationResultMock.Setup(r => r.Data).Returns(operationResultDataMock.Object); + operationResultMock.Setup(r => r.Errors).Returns(new List()); + + _personByCriteriaQueryMock + .Setup(q => q.ExecuteAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(operationResultMock.Object); + + var matchResult = new Shared.Models.MatchResult + { + MatchStatus = Shared.Models.MatchStatus.Match, + NhsNumber = "9999999999", + Score = 1.0m + }; + var apiResult = new Shared.Models.PersonMatchResponse + { + Result = matchResult + }; + + var matchedRecord = new ProcessedMatchRecord + { + OriginalData = new CsvRecordDto(new Dictionary { { "SourceID", "person-123" } }), + ApiResult = apiResult, + IsSuccess = true + }; + + _matchPersonRecordOrchestratorMock + .Setup(o => o.ProcessAsync(It.IsAny>(), It.IsAny(), It.IsAny())) + .ReturnsAsync(new List> { matchedRecord }); + + // Act + await sut.RunAsync(CancellationToken.None); + + // Assert - Verify that ExecuteAsync is NEVER called because the person already has an NHS number + updatePersonMutationMock.Verify( + m => m.ExecuteAsync(It.IsAny(), It.IsAny()), + Times.Never); + } } \ No newline at end of file diff --git a/tests/tools/FakeEclipseGraphQLApi/Models/Person.cs b/tests/tools/FakeEclipseGraphQLApi/Models/Person.cs index ef0296de..a8a5b496 100644 --- a/tests/tools/FakeEclipseGraphQLApi/Models/Person.cs +++ b/tests/tools/FakeEclipseGraphQLApi/Models/Person.cs @@ -12,6 +12,7 @@ public class Person : IPersonByCriteria_PersonByCriteria_Results public string? Gender { get; set; } public string? NhsNumber { get; set; } public int ObjectVersion { get; set; } = 1; + public List PersonTypes { get; set; } = new() { PersonType.CLIENT }; public DateRange? DateOfBirth { get; set; } public List
Addresses { get; set; } = new(); public Address? PreferredAddress { get; set; } diff --git a/tests/tools/FakeEclipseGraphQLApi/Models/PersonType.cs b/tests/tools/FakeEclipseGraphQLApi/Models/PersonType.cs new file mode 100644 index 00000000..f770f486 --- /dev/null +++ b/tests/tools/FakeEclipseGraphQLApi/Models/PersonType.cs @@ -0,0 +1,10 @@ +namespace FakeEclipseGraphQLApi.Models; + +public enum PersonType +{ + ADOPTER, + CLIENT, + FOSTER_CARER, + OTHER, + PROFESSIONAL +} \ No newline at end of file diff --git a/tests/tools/FakeEclipseGraphQLApi/Models/UpdatePerson.cs b/tests/tools/FakeEclipseGraphQLApi/Models/UpdatePerson.cs index d822bb2c..29280256 100644 --- a/tests/tools/FakeEclipseGraphQLApi/Models/UpdatePerson.cs +++ b/tests/tools/FakeEclipseGraphQLApi/Models/UpdatePerson.cs @@ -14,4 +14,6 @@ public class UpdatePerson public string? NhsNumber { get; set; } public int ObjectVersion { get; set; } + + public List? PersonTypes { get; set; } } \ No newline at end of file From d2525b57bf24a0ae5f574669cd1738ab65deb7ee Mon Sep 17 00:00:00 2001 From: Tom Hawkin Date: Wed, 15 Jul 2026 12:18:47 +0100 Subject: [PATCH 3/4] fixed tests --- .../Client/GraphQlProcessJob/GraphQlProcessorTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/Unit.Tests/Client/GraphQlProcessJob/GraphQlProcessorTests.cs b/tests/Unit.Tests/Client/GraphQlProcessJob/GraphQlProcessorTests.cs index 3ed6f52f..b70b0f5c 100644 --- a/tests/Unit.Tests/Client/GraphQlProcessJob/GraphQlProcessorTests.cs +++ b/tests/Unit.Tests/Client/GraphQlProcessJob/GraphQlProcessorTests.cs @@ -95,7 +95,7 @@ public async Task RunAsync_ShouldProcessSinglePage_WhenNoMoreResultsExist() operationResultMock.Setup(r => r.Errors).Returns(new List()); _personByCriteriaQueryMock - .Setup(q => q.ExecuteAsync(25, It.Is(r => r.PageNumber == 1 && r.PageSize == 10), + .Setup(q => q.ExecuteAsync(25, It.Is(r => r.PageNumber == 1 && r.PageSize == 100), It.IsAny())) .ReturnsAsync(operationResultMock.Object); @@ -198,12 +198,12 @@ public async Task RunAsync_ShouldProcessMultiplePages_WhenMoreResultsExist() // Setup query mock to return Page 1 first, then Page 2 _personByCriteriaQueryMock - .Setup(q => q.ExecuteAsync(25, It.Is(r => r.PageNumber == 1 && r.PageSize == 10), + .Setup(q => q.ExecuteAsync(25, It.Is(r => r.PageNumber == 1 && r.PageSize == 100), It.IsAny())) .ReturnsAsync(operationResultMock1.Object); _personByCriteriaQueryMock - .Setup(q => q.ExecuteAsync(25, It.Is(r => r.PageNumber == 2 && r.PageSize == 10), + .Setup(q => q.ExecuteAsync(25, It.Is(r => r.PageNumber == 2 && r.PageSize == 100), It.IsAny())) .ReturnsAsync(operationResultMock2.Object); @@ -278,7 +278,7 @@ public async Task RunAsync_ShouldSkipResult_WhenResultIsNotPerson() operationResultMock.Setup(r => r.Errors).Returns(new List()); _personByCriteriaQueryMock - .Setup(q => q.ExecuteAsync(25, It.Is(r => r.PageNumber == 1 && r.PageSize == 10), + .Setup(q => q.ExecuteAsync(25, It.Is(r => r.PageNumber == 1 && r.PageSize == 100), It.IsAny())) .ReturnsAsync(operationResultMock.Object); From 6b20bcead8e1146d44d921b17ea91f862cc83377 Mon Sep 17 00:00:00 2001 From: Tom Hawkin Date: Wed, 15 Jul 2026 14:15:09 +0100 Subject: [PATCH 4/4] removed personMetaData record and added data to csvRecordDto --- .../Infrastructure/GraphQLProcessor.cs | 53 +++++++++++-------- .../GraphQlProcessorTests.cs | 16 +++++- tests/tools/FakeEclipseGraphQLApi/Mutation.cs | 2 + .../data/personByCriteria.json | 46 +++++++++++++--- 4 files changed, 84 insertions(+), 33 deletions(-) diff --git a/src/SUI.Client/SUI.Client.GraphQLProcessJob/Infrastructure/GraphQLProcessor.cs b/src/SUI.Client/SUI.Client.GraphQLProcessJob/Infrastructure/GraphQLProcessor.cs index bad1d82f..68b83920 100644 --- a/src/SUI.Client/SUI.Client.GraphQLProcessJob/Infrastructure/GraphQLProcessor.cs +++ b/src/SUI.Client/SUI.Client.GraphQLProcessJob/Infrastructure/GraphQLProcessor.cs @@ -21,15 +21,13 @@ public class GraphQlProcessor( IOptions options, IOptions csvMatchDataOptions) { - private record PersonMetadata(int ObjectVersion, string? NhsNumber, IReadOnlyList PersonTypes); - public async Task RunAsync(CancellationToken cancellationToken) { var timer = Stopwatch.StartNew(); logger.LogInformation("Running Graph QL Process Job."); var mappings = csvMatchDataOptions.Value.ColumnMappings; - (List csvRecords, Dictionary personMetadata) = await FetchAndCompilePersonRecordsAsync(mappings, cancellationToken); + List csvRecords = await FetchAndCompilePersonRecordsAsync(mappings, cancellationToken); logger.LogInformation("Completed compiling GraphQL records. Total records retrieved: {Count}. Elapsed Time: {ElapsedTime}", csvRecords.Count, timer.Elapsed.ToString("g")); @@ -47,19 +45,18 @@ public async Task RunAsync(CancellationToken cancellationToken) Result.IsHighConfidenceMatch: true }), timer.Elapsed.ToString("g")); - await SaveMatchedNhsNumbersAsync(matchedResults, personMetadata, mappings, cancellationToken); + await SaveMatchedNhsNumbersAsync(matchedResults, mappings, cancellationToken); logger.LogInformation("GraphQL Job Complete. Total time: {ElapsedTime}", timer.Elapsed.ToString("g")); } - private async Task<(List CsvRecords, Dictionary PersonMetadata)> FetchAndCompilePersonRecordsAsync( + private async Task> FetchAndCompilePersonRecordsAsync( CsvMatchDataOptions.Headers mappings, CancellationToken cancellationToken) { int pageNumber = 1; const int pageSize = 100; var csvRecords = new List(); - var personMetadata = new Dictionary(); while (!cancellationToken.IsCancellationRequested) { @@ -77,11 +74,6 @@ public async Task RunAsync(CancellationToken cancellationToken) { if (result is IPersonByCriteria_PersonByCriteria_Results_Person person) { - personMetadata[person.Id] = new PersonMetadata( - person.ObjectVersion, - person.NhsNumber, - person.PersonTypes ?? [] - ); csvRecords.Add(new CsvRecordDto(MapPersonToDictionary(person, mappings))); } } @@ -102,7 +94,7 @@ public async Task RunAsync(CancellationToken cancellationToken) pageNumber++; } - return (csvRecords, personMetadata); + return csvRecords; } private Dictionary MapPersonToDictionary( @@ -115,7 +107,9 @@ private Dictionary MapPersonToDictionary( { mappings.Given, person.Forename ?? "" }, { mappings.Family, person.Surname ?? "" }, { mappings.BirthDate, person.DateOfBirth?.Lower?.ToString(csvMatchDataOptions.Value.DateFormat) ?? "" }, - { mappings.Postcode, GetPreferredPostcode(person) } + { mappings.Postcode, GetPreferredPostcode(person) }, + { "__ObjectVersion", person.ObjectVersion.ToString() }, + { "__PersonTypes", string.Join(",", person.PersonTypes ?? []) } }; if (!string.IsNullOrEmpty(mappings.NhsNumber)) @@ -136,7 +130,6 @@ private static string GetPreferredPostcode(IPersonByCriteria_PersonByCriteria_Re private async Task SaveMatchedNhsNumbersAsync( IEnumerable> matchedResults, - Dictionary personMetadata, CsvMatchDataOptions.Headers mappings, CancellationToken cancellationToken) { @@ -145,37 +138,51 @@ private async Task SaveMatchedNhsNumbersAsync( if (result.ApiResult is not { Result.IsHighConfidenceMatch: true } || string.IsNullOrEmpty(result.ApiResult.Result.NhsNumber)) { - logger.LogInformation("Match is low confidence, Skipping update."); continue; } var personId = result.OriginalData.Record[mappings.Id]; var matchedNhsNumber = result.ApiResult.Result.NhsNumber; - if (!personMetadata.TryGetValue(personId, out var metadata)) + var existingNhsNumber = !string.IsNullOrEmpty(mappings.NhsNumber) && + result.OriginalData.Record.TryGetValue(mappings.NhsNumber, out var extNhs) + ? extNhs + : null; + + if (!string.IsNullOrEmpty(existingNhsNumber)) { - logger.LogWarning("Could not find metadata for Person {PersonId}. Skipping NHS number update.", personId); + logger.LogInformation("Person {PersonId} already has NHS number. Skipping update.", + personId); continue; } - if (!string.IsNullOrEmpty(metadata.NhsNumber)) + if (!result.OriginalData.Record.TryGetValue("__ObjectVersion", out var objVerStr) || + !int.TryParse(objVerStr, out var objectVersion)) { - logger.LogInformation("Person {PersonId} already has NHS number {ExistingNhsNumber}. Skipping update.", - personId, metadata.NhsNumber); + logger.LogWarning("Could not find ObjectVersion for Person {PersonId}. Skipping NHS number update.", personId); continue; } logger.LogInformation("Saving matched NHS number {NhsNumber} for Person {PersonId} with ObjectVersion {ObjectVersion}.", - matchedNhsNumber, personId, metadata.ObjectVersion); + matchedNhsNumber, personId, objectVersion); try { + var personTypesList = new List(); + if (result.OriginalData.Record.TryGetValue("__PersonTypes", out var typesStr) && + !string.IsNullOrEmpty(typesStr)) + { + personTypesList = typesStr.Split(',', StringSplitOptions.RemoveEmptyEntries) + .Select(Enum.Parse) + .ToList(); + } + var updateInput = new UpdatePerson { Id = personId, NhsNumber = matchedNhsNumber, - ObjectVersion = metadata.ObjectVersion, - PersonTypes = metadata.PersonTypes + ObjectVersion = objectVersion, + PersonTypes = personTypesList }; var updateResult = await eclipseClient.UpdatePerson.ExecuteAsync(updateInput, cancellationToken); diff --git a/tests/Unit.Tests/Client/GraphQlProcessJob/GraphQlProcessorTests.cs b/tests/Unit.Tests/Client/GraphQlProcessJob/GraphQlProcessorTests.cs index b70b0f5c..52fda67a 100644 --- a/tests/Unit.Tests/Client/GraphQlProcessJob/GraphQlProcessorTests.cs +++ b/tests/Unit.Tests/Client/GraphQlProcessJob/GraphQlProcessorTests.cs @@ -922,7 +922,13 @@ public async Task RunAsync_ShouldCallUpdatePersonMutation_WhenHighConfidenceMatc var matchedRecord = new ProcessedMatchRecord { - OriginalData = new CsvRecordDto(new Dictionary { { "SourceID", "person-123" } }), + OriginalData = new CsvRecordDto(new Dictionary + { + { "SourceID", "person-123" }, + { "__ObjectVersion", "5" }, + { "NHSNumber", "" }, + { "__PersonTypes", "Client" } + }), ApiResult = apiResult, IsSuccess = true }; @@ -1021,7 +1027,13 @@ public async Task RunAsync_ShouldNotCallUpdatePersonMutation_WhenPersonAlreadyHa var matchedRecord = new ProcessedMatchRecord { - OriginalData = new CsvRecordDto(new Dictionary { { "SourceID", "person-123" } }), + OriginalData = new CsvRecordDto(new Dictionary + { + { "SourceID", "person-123" }, + { "__ObjectVersion", "5" }, + { "NHSNumber", "1111111111" }, + { "__PersonTypes", "Client" } + }), ApiResult = apiResult, IsSuccess = true }; diff --git a/tests/tools/FakeEclipseGraphQLApi/Mutation.cs b/tests/tools/FakeEclipseGraphQLApi/Mutation.cs index 0aea72c8..af0950d7 100644 --- a/tests/tools/FakeEclipseGraphQLApi/Mutation.cs +++ b/tests/tools/FakeEclipseGraphQLApi/Mutation.cs @@ -41,6 +41,8 @@ public Person UpdatePerson(UpdatePerson input) } person.NhsNumber = input.NhsNumber; + var updatedObjectVersion = person.ObjectVersion + 1; + person.ObjectVersion = updatedObjectVersion; var updatedJson = JsonSerializer.Serialize(response, options); diff --git a/tests/tools/FakeEclipseGraphQLApi/data/personByCriteria.json b/tests/tools/FakeEclipseGraphQLApi/data/personByCriteria.json index d17783d0..a569eeb3 100644 --- a/tests/tools/FakeEclipseGraphQLApi/data/personByCriteria.json +++ b/tests/tools/FakeEclipseGraphQLApi/data/personByCriteria.json @@ -13,7 +13,11 @@ "Forename": "OCTAVIA", "Surname": "CHISLETT", "Gender": "FEMALE", - "NhsNumber": "9449305552", + "NhsNumber": "9691292211", + "ObjectVersion": 3, + "PersonTypes": [ + 1 + ], "DateOfBirth": { "Lower": "2008-09-20", "Upper": "2008-09-20", @@ -28,7 +32,8 @@ } ], "PreferredAddress": { - "Id": "addr-12345678" + "Id": "addr-12345678", + "Location": null } }, { @@ -38,6 +43,10 @@ "Surname": "FORMBY", "Gender": "MALE", "NhsNumber": "9691914913", + "ObjectVersion": 1, + "PersonTypes": [ + 1 + ], "DateOfBirth": { "Lower": "2005-06-03", "Upper": "2005-06-03", @@ -52,7 +61,8 @@ } ], "PreferredAddress": { - "Id": "addr-12323232" + "Id": "addr-12323232", + "Location": null } }, { @@ -62,6 +72,10 @@ "Surname": "FORMBY", "Gender": "MALE", "NhsNumber": "9691914913", + "ObjectVersion": 1, + "PersonTypes": [ + 1 + ], "DateOfBirth": { "Lower": "2005-06-03", "Upper": "2005-06-03", @@ -76,7 +90,8 @@ } ], "PreferredAddress": { - "Id": "addr-12323232-2" + "Id": "addr-12323232-2", + "Location": null } }, { @@ -86,6 +101,10 @@ "Surname": "chislett", "Gender": "MALE", "NhsNumber": "9449310424", + "ObjectVersion": 1, + "PersonTypes": [ + 1 + ], "DateOfBirth": { "Lower": "2008-09-20", "Upper": "2008-09-20", @@ -100,7 +119,8 @@ } ], "PreferredAddress": { - "Id": "addr-34556655" + "Id": "addr-34556655", + "Location": null } }, { @@ -110,6 +130,10 @@ "Surname": "chislett", "Gender": "MALE", "NhsNumber": "0449310999", + "ObjectVersion": 1, + "PersonTypes": [ + 1 + ], "DateOfBirth": { "Lower": "2008-09-20", "Upper": "2008-09-20", @@ -124,7 +148,8 @@ } ], "PreferredAddress": { - "Id": "addr-34567655" + "Id": "addr-34567655", + "Location": null } }, { @@ -134,6 +159,10 @@ "Surname": "Missing MHS Number", "Gender": "MALE", "NhsNumber": "", + "ObjectVersion": 1, + "PersonTypes": [ + 1 + ], "DateOfBirth": { "Lower": "2008-09-20", "Upper": "2008-09-20", @@ -148,8 +177,9 @@ } ], "PreferredAddress": { - "Id": "addr-34568855" + "Id": "addr-34568855", + "Location": null } } ] -} +} \ No newline at end of file