Bug/ulitp 5394 Refactor common land mapping and address selection

src/KeeperData.Application/Orchestration/Imports/Sam/Holdings/Steps/SamHoldingImportSilverMappingStep.cs

@@ -25,7 +25,10 @@ protected override async Task ExecuteCoreAsync(SamHoldingImportContext context,
             countryIdentifierLookupService.FindAsync,
             cancellationToken);
 
-        var commonLandHoldings = SamCommonLandMapper.ToSilver(context.RawCommonLandsByCommonCph);
+        var commonLandHoldings = await SamCommonLandMapper.ToSilver(
+            context.RawCommonLandsByCommonCph,
+            countryIdentifierLookupService.FindAsync,
+            cancellationToken);
         if (commonLandHoldings.Count > 0)
         {
             context.SilverHoldings.AddRange(commonLandHoldings);

src/KeeperData.Application/Orchestration/Imports/Sam/Mappings/SamCommonLandMapper.cs

@@ -10,7 +10,10 @@ public static class SamCommonLandMapper
     private const string CommonLandSiteTypeCode = "CL";
     private const string CommonLandBusinessUsage = "Common Land";
 
-    public static List<SamHoldingDocument> ToSilver(List<SamCommonLand> rawCommonLands)
+    public static async Task<List<SamHoldingDocument>> ToSilver(
+        List<SamCommonLand> rawCommonLands,
+        Func<string?, string?, CancellationToken, Task<(string? countryId, string? countryCode, string? countryName)>> resolveCountry,
+        CancellationToken cancellationToken)
     {
         if (rawCommonLands == null || rawCommonLands.Count == 0)
             return [];
@@ -33,6 +36,8 @@ public static List<SamHoldingDocument> ToSilver(List<SamCommonLand> rawCommonLan
                 });
             }
 
+            var (countryId, countryCode, _) = await resolveCountry(representative.COUNTRY, null, cancellationToken);
+
             var holding = new SamHoldingDocument
             {
                 LastUpdatedBatchId = representative.BATCH_ID,
@@ -64,10 +69,11 @@ public static List<SamHoldingDocument> ToSilver(List<SamCommonLand> rawCommonLan
                     {
                         IdentifierId = Guid.NewGuid().ToString(),
                         AddressLine = representative.ADDRESS_LINE_1,
-                        AddressLocality = representative.ADDRESS_LINE_2,
-                        AddressStreet = representative.ADDRESS_LINE_3,
+                        AddressStreet = representative.ADDRESS_LINE_2,
+                        AddressTown = representative.ADDRESS_LINE_3,
                         AddressPostCode = representative.POSTCODE,
-                        CountryCode = representative.COUNTRY
+                        CountryCode = countryCode,
+                        CountryIdentifier = countryId
                     }
                 }
             };

src/KeeperData.Application/Orchestration/Imports/Sam/Mappings/SamHoldingMapper.cs

@@ -165,6 +165,19 @@ internal static SamHoldingDocument SelectRepresentativeHolding(List<SamHoldingDo
         return silverHoldings.OrderByDescending(h => h.LastUpdatedDate).First();
     }
 
+    public static SamHoldingDocument SelectAddressSource(List<SamHoldingDocument> silverHoldings)
+    {
+        const string commonLandBusinessUsage = "Common Land";
+
+        // Common land address takes precedence — use the most recently updated common land if present
+        var commonLand = silverHoldings
+            .Where(x => x.SourceFacilitySubBusinessActivityCode == commonLandBusinessUsage)
+            .OrderByDescending(h => h.LastUpdatedDate)
+            .FirstOrDefault();
+
+        return commonLand ?? SelectRepresentativeHolding(silverHoldings);
+    }
+
     public static async Task<SiteDocument?> ToGold(
         string goldSiteId,
         SiteDocument? existingSite,
@@ -185,6 +198,9 @@ internal static SamHoldingDocument SelectRepresentativeHolding(List<SamHoldingDo
         // Prefer SAM Holding over Common Land when selecting representative
         var representative = SelectRepresentativeHolding(silverHoldings);
 
+        // Common land address takes precedence over site address for location data
+        var addressSource = SelectAddressSource(silverHoldings);
+
         var distinctSpecies = await GetDistinctReferenceDataAsync(
             silverHoldings.Select(h => h.SpeciesTypeCode),
             findSpecies,
@@ -220,6 +236,7 @@ internal static SamHoldingDocument SelectRepresentativeHolding(List<SamHoldingDo
         var site = existingSite is not null
             ? await UpdateSiteAsync(
                 representative,
+                addressSource,
                 existingSite,
                 goldSiteGroupMarks,
                 goldParties,
@@ -232,6 +249,7 @@ internal static SamHoldingDocument SelectRepresentativeHolding(List<SamHoldingDo
             : await CreateSiteAsync(
                 goldSiteId,
                 representative,
+                addressSource,
                 goldSiteGroupMarks,
                 goldParties,
                 getCountryById,
@@ -341,6 +359,7 @@ private static bool IsActivityAlreadyAdded(List<SiteActivity> activities, string
     private static async Task<Site> CreateSiteAsync(
         string goldSiteId,
         SamHoldingDocument representative,
+        SamHoldingDocument addressSource,
         List<SiteGroupMarkRelationshipDocument> goldSiteGroupMarks,
         List<PartyDocument> goldParties,
         Func<string?, CancellationToken, Task<CountryDocument?>> getCountryById,
@@ -350,13 +369,13 @@ private static async Task<Site> CreateSiteAsync(
         SiteIdentifierType? siteIdentifierType,
         CancellationToken cancellationToken)
     {
-        var (address, communication) = await ResolveLocationPartsAsync(representative, getCountryById, cancellationToken);
+        var (address, communication) = await ResolveLocationPartsAsync(addressSource, getCountryById, cancellationToken);
         var isPermanentLandHolding = representative.CphRelationshipType.IsPermanentLandHolding();
 
         var location = Location.Create(
-            representative.Location?.OsMapReference,
-            representative.Location?.Easting,
-            representative.Location?.Northing,
+            addressSource.Location?.OsMapReference,
+            addressSource.Location?.Easting,
+            addressSource.Location?.Northing,
             address,
             communication: [communication]);
 
@@ -384,6 +403,7 @@ private static async Task<Site> CreateSiteAsync(
 
     private static async Task<Site> UpdateSiteAsync(
         SamHoldingDocument representative,
+        SamHoldingDocument addressSource,
         SiteDocument existing,
         List<SiteGroupMarkRelationshipDocument> goldSiteGroupMarks,
         List<PartyDocument> goldParties,
@@ -410,16 +430,16 @@ private static async Task<Site> UpdateSiteAsync(
             representative.CphTypeIdentifier,
             isPermanentLandHolding ? representative.SecondaryCph : null);
 
-        var (updatedAddress, updatedCommunication) = await ResolveLocationPartsAsync(representative, getCountryById, cancellationToken);
+        var (updatedAddress, updatedCommunication) = await ResolveLocationPartsAsync(addressSource, getCountryById, cancellationToken);
 
         // Always set the derived site type (may be null if no mapping found).
         site.SetSiteType(siteType, representative.LastUpdatedDate);
 
         site.SetLocation(
             representative.LastUpdatedDate,
-            representative.Location?.OsMapReference,
-            representative.Location?.Easting,
-            representative.Location?.Northing,
+            addressSource.Location?.OsMapReference,
+            addressSource.Location?.Easting,
+            addressSource.Location?.Northing,
             updatedAddress,
             [updatedCommunication]);
 

tests/KeeperData.Application.Tests.Unit/Orchestration/Imports/Sam/Mappings/SamCommonLandMapperTests.cs

@@ -9,34 +9,40 @@ namespace KeeperData.Application.Tests.Unit.Orchestration.Imports.Sam.Mappings;
 
 public class SamCommonLandMapperTests
 {
+    private static readonly Func<string?, string?, CancellationToken, Task<(string?, string?, string?)>> NoopResolveCountry =
+        (_, _, _) => Task.FromResult<(string?, string?, string?)>((null, null, null));
+
+    private static Func<string?, string?, CancellationToken, Task<(string?, string?, string?)>> ResolveCountry(string? id, string? code) =>
+        (_, _, _) => Task.FromResult<(string?, string?, string?)>((id, code, null));
+
     [Fact]
-    public void ToSilver_WithNullInput_ShouldReturnEmptyList()
+    public async Task ToSilver_WithNullInput_ShouldReturnEmptyList()
     {
         // Arrange
         List<SamCommonLand>? rawCommonLands = null;
 
         // Act
-        var result = SamCommonLandMapper.ToSilver(rawCommonLands!);
+        var result = await SamCommonLandMapper.ToSilver(rawCommonLands!, NoopResolveCountry, CancellationToken.None);
 
         // Assert
         result.Should().BeEmpty();
     }
 
     [Fact]
-    public void ToSilver_WithEmptyList_ShouldReturnEmptyList()
+    public async Task ToSilver_WithEmptyList_ShouldReturnEmptyList()
     {
         // Arrange
         var rawCommonLands = new List<SamCommonLand>();
 
         // Act
-        var result = SamCommonLandMapper.ToSilver(rawCommonLands);
+        var result = await SamCommonLandMapper.ToSilver(rawCommonLands, NoopResolveCountry, CancellationToken.None);
 
         // Assert
         result.Should().BeEmpty();
     }
 
     [Fact]
-    public void ToSilver_WithDefinitionRecordOnly_ShouldCreateSamHoldingDocument()
+    public async Task ToSilver_WithDefinitionRecordOnly_ShouldCreateSamHoldingDocument()
     {
         // Arrange
         var now = DateTime.UtcNow;
@@ -68,7 +74,10 @@ public void ToSilver_WithDefinitionRecordOnly_ShouldCreateSamHoldingDocument()
         };
 
         // Act
-        var result = SamCommonLandMapper.ToSilver(rawCommonLands);
+        var result = await SamCommonLandMapper.ToSilver(
+            rawCommonLands,
+            ResolveCountry("country-id-1", "England"),
+            CancellationToken.None);
 
         // Assert
         result.Should().HaveCount(1);
@@ -88,14 +97,15 @@ public void ToSilver_WithDefinitionRecordOnly_ShouldCreateSamHoldingDocument()
         holding.Location.Northing.Should().Be(569204);
         holding.Location.Address.Should().NotBeNull();
         holding.Location.Address!.AddressLine.Should().Be("Land off Road");
-        holding.Location.Address.AddressLocality.Should().Be("Village");
-        holding.Location.Address.AddressStreet.Should().Be("District");
+        holding.Location.Address.AddressStreet.Should().Be("Village");
+        holding.Location.Address.AddressTown.Should().Be("District");
         holding.Location.Address.AddressPostCode.Should().Be("AB12 3CD");
         holding.Location.Address.CountryCode.Should().Be("England");
+        holding.Location.Address.CountryIdentifier.Should().Be("country-id-1");
     }
 
     [Fact]
-    public void ToSilver_WithDeletedRecord_ShouldMapDeletedStatus()
+    public async Task ToSilver_WithDeletedRecord_ShouldMapDeletedStatus()
     {
         // Arrange
         var rawCommonLands = new List<SamCommonLand>
@@ -112,7 +122,7 @@ public void ToSilver_WithDeletedRecord_ShouldMapDeletedStatus()
         };
 
         // Act
-        var result = SamCommonLandMapper.ToSilver(rawCommonLands);
+        var result = await SamCommonLandMapper.ToSilver(rawCommonLands, NoopResolveCountry, CancellationToken.None);
 
         // Assert
         result.Should().HaveCount(1);
@@ -121,7 +131,7 @@ public void ToSilver_WithDeletedRecord_ShouldMapDeletedStatus()
     }
 
     [Fact]
-    public void ToSilver_WithPlaceholderPremisesName_ShouldSetToNull()
+    public async Task ToSilver_WithPlaceholderPremisesName_ShouldSetToNull()
     {
         // Arrange
         var rawCommonLands = new List<SamCommonLand>
@@ -136,14 +146,14 @@ public void ToSilver_WithPlaceholderPremisesName_ShouldSetToNull()
         };
 
         // Act
-        var result = SamCommonLandMapper.ToSilver(rawCommonLands);
+        var result = await SamCommonLandMapper.ToSilver(rawCommonLands, NoopResolveCountry, CancellationToken.None);
 
         // Assert
         result[0].LocationName.Should().BeNull();
     }
 
     [Fact]
-    public void ToSilver_WithEmptyCommonCph_ShouldBeFiltered()
+    public async Task ToSilver_WithEmptyCommonCph_ShouldBeFiltered()
     {
         // Arrange
         var rawCommonLands = new List<SamCommonLand>
@@ -163,14 +173,14 @@ public void ToSilver_WithEmptyCommonCph_ShouldBeFiltered()
         };
 
         // Act
-        var result = SamCommonLandMapper.ToSilver(rawCommonLands);
+        var result = await SamCommonLandMapper.ToSilver(rawCommonLands, NoopResolveCountry, CancellationToken.None);
 
         // Assert
         result.Should().BeEmpty();
     }
 
     [Fact]
-    public void ToSilver_WithInvalidEastingNorthing_ShouldSetToNull()
+    public async Task ToSilver_WithInvalidEastingNorthing_ShouldSetToNull()
     {
         // Arrange
         var rawCommonLands = new List<SamCommonLand>
@@ -187,15 +197,15 @@ public void ToSilver_WithInvalidEastingNorthing_ShouldSetToNull()
         };
 
         // Act
-        var result = SamCommonLandMapper.ToSilver(rawCommonLands);
+        var result = await SamCommonLandMapper.ToSilver(rawCommonLands, NoopResolveCountry, CancellationToken.None);
 
         // Assert
         result[0].Location!.Easting.Should().BeNull();
         result[0].Location!.Northing.Should().BeNull();
     }
 
     [Fact]
-    public void ToSilver_WithFutureDate_ShouldNormaliseToNull()
+    public async Task ToSilver_WithFutureDate_ShouldNormaliseToNull()
     {
         // Arrange
         var rawCommonLands = new List<SamCommonLand>
@@ -216,7 +226,7 @@ public void ToSilver_WithFutureDate_ShouldNormaliseToNull()
         };
 
         // Act
-        var result = SamCommonLandMapper.ToSilver(rawCommonLands);
+        var result = await SamCommonLandMapper.ToSilver(rawCommonLands, NoopResolveCountry, CancellationToken.None);
 
         // Assert
         result[1].AssociatedMainHoldings[0].StartDate.Should().BeNull();
@@ -376,7 +386,7 @@ public void ToAssociatedCommonLands_WithVariousDateFormats_ShouldNormaliseCorrec
     }
 
     [Fact]
-    public void ToSilver_ShouldMapBusinessUsageToSourceFacilitySubBusinessActivityCode()
+    public async Task ToSilver_ShouldMapBusinessUsageToSourceFacilitySubBusinessActivityCode()
     {
         // Arrange
         var rawCommonLands = new List<SamCommonLand>
@@ -392,15 +402,15 @@ public void ToSilver_ShouldMapBusinessUsageToSourceFacilitySubBusinessActivityCo
         };
 
         // Act
-        var result = SamCommonLandMapper.ToSilver(rawCommonLands);
+        var result = await SamCommonLandMapper.ToSilver(rawCommonLands, NoopResolveCountry, CancellationToken.None);
 
         // Assert
         result.Should().HaveCount(1);
         result[0].SourceFacilitySubBusinessActivityCode.Should().Be("Common Land");
     }
 
     [Fact]
-    public void ToSilver_ShouldMapSiteTypeCodeToCL()
+    public async Task ToSilver_ShouldMapSiteTypeCodeToCL()
     {
         // Arrange
         var rawCommonLands = new List<SamCommonLand>
@@ -416,7 +426,7 @@ public void ToSilver_ShouldMapSiteTypeCodeToCL()
         };
 
         // Act
-        var result = SamCommonLandMapper.ToSilver(rawCommonLands);
+        var result = await SamCommonLandMapper.ToSilver(rawCommonLands, NoopResolveCountry, CancellationToken.None);
 
         // Assert
         result.Should().HaveCount(1);