Skip to content

Bug/ulitp 5394 #237

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
grahammcvea-defra merged 8 commits into from
Jun 10, 2026
Merged

Bug/ulitp 5394 #237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2110582
Refactor holding selection and merge logic for SAM import
Bartonkeys Jun 9, 2026
28b8232
Merge branch 'dev' into bug/ulitp-5394
Bartonkeys Jun 9, 2026
bdca9b4
Refactor site data mapping; add Common Land test
Bartonkeys Jun 9, 2026
a39a47d
Remove unused capturedFilter variable from test class
Bartonkeys Jun 10, 2026
70a37b4
Ensure ProductionUsageCodeList contains unique values
Bartonkeys Jun 10, 2026
3fd70c8
Refactor address/comm mapping with helper method
Bartonkeys Jun 10, 2026
87dbda0
Move SelectRepresentativeHolding to SamHoldingMapper
Bartonkeys Jun 10, 2026
c61dfd0
Whitespace
Bartonkeys Jun 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 20 additions & 10 deletions ...a.Application/Orchestration/Imports/Sam/Holdings/Steps/SamHoldingImportGoldMappingStep.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,8 @@
{
if (context.SilverHoldings.Count > 0)
{
var representative = context.SilverHoldings.Any(x => x.HoldingStatus == HoldingStatusType.Active.GetDescription())
? context.SilverHoldings.Where(x => x.HoldingStatus == HoldingStatusType.Active.GetDescription()).OrderByDescending(h => h.LastUpdatedDate).First()
: context.SilverHoldings.OrderByDescending(h => h.LastUpdatedDate).First();
// Prefer SAM Holding over Common Land when selecting representative
var representative = SamHoldingMapper.SelectRepresentativeHolding(context.SilverHoldings);

var existingHoldingFilter = Builders<SiteDocument>.Filter.ElemMatch(
x => x.Identifiers,
Expand Down Expand Up @@ -69,7 +68,7 @@
siteTypeDerivedCodeLookupService,
cancellationToken);

await EnrichWithCommonLandDataAsync(context, representative, cancellationToken);
await EnrichWithCommonLandDataAsync(context, context.SilverHoldings, cancellationToken);

logger.LogInformation("Associated main sites queued for update: {Count} for CPH {Cph}",
context.AssociatedMainSites?.Count ?? 0, context.Cph);
Expand All @@ -86,14 +85,21 @@
}
}

private async Task EnrichWithCommonLandDataAsync(SamHoldingImportContext context, SamHoldingDocument representative, CancellationToken cancellationToken)
private async Task EnrichWithCommonLandDataAsync(SamHoldingImportContext context, List<SamHoldingDocument> silverHoldings, CancellationToken cancellationToken)
{
var goldSite = context.GoldSite;
if (goldSite == null) return;

goldSite.LocalAuthorityName = representative.LocalAuthorityName;
// Merge LocalAuthorityName - prefer non-null value from any holding
goldSite.LocalAuthorityName = silverHoldings
.Select(h => h.LocalAuthorityName)
.FirstOrDefault(name => !string.IsNullOrWhiteSpace(name));

goldSite.AssociatedMainHoldings = representative.AssociatedMainHoldings
// Merge AssociatedMainHoldings from all holdings, removing duplicates
var allMainHoldings = silverHoldings
.SelectMany(h => h.AssociatedMainHoldings)
.GroupBy(r => r.HoldingIdentifier)
.Select(g => g.OrderByDescending(r => r.StartDate).First())
.Select(r => new AssociatedHoldingDocument
{
HoldingIdentifier = r.HoldingIdentifier,
Expand All @@ -103,13 +109,17 @@
})
.ToList();

goldSite.AssociatedMainHoldings = allMainHoldings;

if (goldSite.AssociatedMainHoldings?.Count > 0)
{
await FindAndUpdateMainSiteIfExists(context, representative, goldSite.AssociatedMainHoldings, cancellationToken);
// Get the CPH from any holding (they all have the same CPH)
var cph = silverHoldings.First().CountyParishHoldingNumber;

Check warning on line 117 in src/KeeperData.Application/Orchestration/Imports/Sam/Holdings/Steps/SamHoldingImportGoldMappingStep.cs

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Indexing at 0 should be used instead of the "Enumerable" extension method "First" 


See more on https://sonarcloud.io/project/issues?id=DEFRA_ls-keeper-data-api&issues=AZ6rJmNWlZga0k7GEqYH&open=AZ6rJmNWlZga0k7GEqYH&pullRequest=237
await FindAndUpdateMainSiteIfExists(context, cph, goldSite.AssociatedMainHoldings, cancellationToken);
}
}

private async Task FindAndUpdateMainSiteIfExists(SamHoldingImportContext context, SamHoldingDocument representative, List<AssociatedHoldingDocument> mainHoldings, CancellationToken cancellationToken)
private async Task FindAndUpdateMainSiteIfExists(SamHoldingImportContext context, string commonCph, List<AssociatedHoldingDocument> mainHoldings, CancellationToken cancellationToken)
{
foreach (var mainHolding in mainHoldings)
{
Expand All @@ -132,7 +142,7 @@

var commonForMain = new AssociatedHoldingDocument
{
HoldingIdentifier = representative.CountyParishHoldingNumber,
HoldingIdentifier = commonCph,
ContiguousFlag = mainHolding.ContiguousFlag,
StartDate = mainHolding.StartDate,
EndDate = mainHolding.EndDate
Expand Down
145 changes: 91 additions & 54 deletions src/KeeperData.Application/Orchestration/Imports/Sam/Mappings/SamHoldingMapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public static async Task<SamHoldingDocument> ToSilver(
SiteTypeCode = null,

SpeciesTypeCode = h.AnimalSpeciesCodeUnwrapped,
ProductionUsageCodeList = [.. h.AnimalProductionUsageCodeList.Select(ProductionUsageCodeFormatters.TrimProductionUsageCodeHolding)],
ProductionUsageCodeList = [.. h.AnimalProductionUsageCodeList.Select(ProductionUsageCodeFormatters.TrimProductionUsageCodeHolding).Distinct()],

Location = new Core.Documents.Silver.LocationDocument
{
Expand Down Expand Up @@ -129,6 +129,42 @@ public static async Task<SamHoldingDocument> ToSilver(
return result;
}

internal static SamHoldingDocument SelectRepresentativeHolding(List<SamHoldingDocument> silverHoldings)
{
const string commonLandBusinessUsage = "Common Land";
var activeStatus = HoldingStatusType.Active.GetDescription();

// Priority 1: Active SAM Holding (not Common Land)
var activeSamHolding = silverHoldings
.Where(x => x.HoldingStatus == activeStatus && x.SourceFacilitySubBusinessActivityCode != commonLandBusinessUsage)
.OrderByDescending(h => h.LastUpdatedDate)
.FirstOrDefault();

if (activeSamHolding != null)
return activeSamHolding;

// Priority 2: Any SAM Holding (not Common Land)
var samHolding = silverHoldings
.Where(x => x.SourceFacilitySubBusinessActivityCode != commonLandBusinessUsage)
.OrderByDescending(h => h.LastUpdatedDate)
.FirstOrDefault();

if (samHolding != null)
return samHolding;

// Priority 3: Active Common Land
var activeCommonLand = silverHoldings
.Where(x => x.HoldingStatus == activeStatus)
.OrderByDescending(h => h.LastUpdatedDate)
.FirstOrDefault();

if (activeCommonLand != null)
return activeCommonLand;

// Priority 4: Any holding (fallback)
return silverHoldings.OrderByDescending(h => h.LastUpdatedDate).First();
}

public static async Task<SiteDocument?> ToGold(
string goldSiteId,
SiteDocument? existingSite,
Expand All @@ -146,9 +182,8 @@ public static async Task<SamHoldingDocument> ToSilver(
if (silverHoldings == null || silverHoldings.Count == 0)
return null;

var representative = silverHoldings.Any(x => x.HoldingStatus == HoldingStatusType.Active.GetDescription())
? silverHoldings.Where(x => x.HoldingStatus == HoldingStatusType.Active.GetDescription()).OrderByDescending(h => h.LastUpdatedDate).First()
: silverHoldings.OrderByDescending(h => h.LastUpdatedDate).First();
// Prefer SAM Holding over Common Land when selecting representative
var representative = SelectRepresentativeHolding(silverHoldings);

var distinctSpecies = await GetDistinctReferenceDataAsync(
silverHoldings.Select(h => h.SpeciesTypeCode),
Expand Down Expand Up @@ -315,8 +350,8 @@ private static async Task<Site> CreateSiteAsync(
SiteIdentifierType? siteIdentifierType,
CancellationToken cancellationToken)
{
var address = await LocationMapper.AddressToGold(representative.Location?.Address, getCountryById, cancellationToken);
var communication = LocationMapper.CommunicationToGold(representative.Communication);
var (address, communication) = await ResolveLocationPartsAsync(representative, getCountryById, cancellationToken);
var isPermanentLandHolding = representative.CphRelationshipType.IsPermanentLandHolding();

var location = Location.Create(
representative.Location?.OsMapReference,
Expand All @@ -325,13 +360,6 @@ private static async Task<Site> CreateSiteAsync(
address,
communication: [communication]);

var groupMarks = ToGroupMarks(goldSiteGroupMarks);

var siteParties = goldParties
.Where(p => !p.Deleted && !string.IsNullOrWhiteSpace(p.CustomerNumber))
.Select(p => p.ToSitePartyDomain(representative.LastUpdatedDate))
.ToList();

var site = Site.Create(
goldSiteId,
representative.CreatedDate,
Expand All @@ -343,26 +371,13 @@ private static async Task<Site> CreateSiteAsync(
SourceSystemType.SAM.ToString(),
null,
representative.Deleted,
representative.CphRelationshipType.IsPermanentLandHolding() ? null : representative.SecondaryCph,
isPermanentLandHolding ? null : representative.SecondaryCph,
representative.CphTypeIdentifier,
siteType,
location,
representative.CphRelationshipType.IsPermanentLandHolding() ? representative.SecondaryCph : null);

if (siteIdentifierType != null)
{
site.SetSiteIdentifier(
identifierLastUpdatedDate: representative.LastUpdatedDate,
identifier: representative.CountyParishHoldingNumber,
type: siteIdentifierType,
id: null,
siteLastUpdatedDate: representative.LastUpdatedDate);
}
isPermanentLandHolding ? representative.SecondaryCph : null);

site.SetSpecies(species, representative.LastUpdatedDate);
site.SetActivities(activities, representative.LastUpdatedDate);
site.SetGroupMarks(groupMarks, representative.LastUpdatedDate);
site.SetSiteParties(goldSiteId, siteParties, representative.LastUpdatedDate);
ApplySiteData(site, goldSiteId, representative, goldSiteGroupMarks, goldParties, species, activities, siteIdentifierType);

return site;
}
Expand All @@ -379,15 +394,9 @@ private static async Task<Site> UpdateSiteAsync(
SiteIdentifierType? siteIdentifierType,
CancellationToken cancellationToken)
{
var isPermanentLandHolding = representative.CphRelationshipType.IsPermanentLandHolding();
var site = existing.ToDomain();

var groupMarks = ToGroupMarks(goldSiteGroupMarks);

var siteParties = goldParties
.Where(p => !p.Deleted && !string.IsNullOrWhiteSpace(p.CustomerNumber))
.Select(p => p.ToSitePartyDomain(representative.LastUpdatedDate))
.ToList();

site.Update(
representative.LastUpdatedDate,
representative.LocationName ?? string.Empty,
Expand All @@ -397,12 +406,11 @@ private static async Task<Site> UpdateSiteAsync(
SourceSystemType.SAM.ToString(),
null,
representative.Deleted,
representative.CphRelationshipType.IsPermanentLandHolding() ? null : representative.SecondaryCph,
isPermanentLandHolding ? null : representative.SecondaryCph,
representative.CphTypeIdentifier,
representative.CphRelationshipType.IsPermanentLandHolding() ? representative.SecondaryCph : null);
isPermanentLandHolding ? representative.SecondaryCph : null);

var updatedAddress = await LocationMapper.AddressToGold(representative.Location?.Address, getCountryById, cancellationToken);
var updatedCommunication = LocationMapper.CommunicationToGold(representative.Communication);
var (updatedAddress, updatedCommunication) = await ResolveLocationPartsAsync(representative, getCountryById, cancellationToken);

// Always set the derived site type (may be null if no mapping found).
site.SetSiteType(siteType, representative.LastUpdatedDate);
Expand All @@ -415,25 +423,22 @@ private static async Task<Site> UpdateSiteAsync(
updatedAddress,
[updatedCommunication]);

if (siteIdentifierType != null)
{
site.SetSiteIdentifier(
identifierLastUpdatedDate: representative.LastUpdatedDate,
identifier: representative.CountyParishHoldingNumber,
type: siteIdentifierType,
id: null,
siteLastUpdatedDate: representative.LastUpdatedDate);
}

site.SetSpecies(species, representative.LastUpdatedDate);
site.SetActivities(activities, representative.LastUpdatedDate);
site.SetGroupMarks(groupMarks, representative.LastUpdatedDate);
site.SetSiteParties(existing.Id, siteParties, representative.LastUpdatedDate);
ApplySiteData(site, existing.Id, representative, goldSiteGroupMarks, goldParties, species, activities, siteIdentifierType);

return site;
}


private static async Task<(Address address, Communication communication)> ResolveLocationPartsAsync(
SamHoldingDocument representative,
Func<string?, CancellationToken, Task<CountryDocument?>> getCountryById,
CancellationToken cancellationToken)
{
var address = await LocationMapper.AddressToGold(representative.Location?.Address, getCountryById, cancellationToken);
var communication = LocationMapper.CommunicationToGold(representative.Communication);
return (address, communication);
}

private static async Task<List<(string searchValue, string? typeId, string? typeName)>> GetDistinctReferenceDataAsync(
IEnumerable<string?> rawCodes,
Func<string?, CancellationToken, Task<(string? typeId, string? typeName)>> findAsync,
Expand All @@ -455,6 +460,38 @@ private static async Task<Site> UpdateSiteAsync(
return [.. results];
}

private static void ApplySiteData(
Site site,
string siteId,
SamHoldingDocument representative,
List<SiteGroupMarkRelationshipDocument> goldSiteGroupMarks,
List<PartyDocument> goldParties,
List<Species> species,
List<SiteActivity> activities,
SiteIdentifierType? siteIdentifierType)
{
var groupMarks = ToGroupMarks(goldSiteGroupMarks);
var siteParties = goldParties
.Where(p => !p.Deleted && !string.IsNullOrWhiteSpace(p.CustomerNumber))
.Select(p => p.ToSitePartyDomain(representative.LastUpdatedDate))
.ToList();

if (siteIdentifierType != null)
{
site.SetSiteIdentifier(
identifierLastUpdatedDate: representative.LastUpdatedDate,
identifier: representative.CountyParishHoldingNumber,
type: siteIdentifierType,
id: null,
siteLastUpdatedDate: representative.LastUpdatedDate);
}

site.SetSpecies(species, representative.LastUpdatedDate);
site.SetActivities(activities, representative.LastUpdatedDate);
site.SetGroupMarks(groupMarks, representative.LastUpdatedDate);
site.SetSiteParties(siteId, siteParties, representative.LastUpdatedDate);
}

private static List<GroupMark> ToGroupMarks(List<SiteGroupMarkRelationshipDocument> relationships)
{
return
Expand Down
44 changes: 37 additions & 7 deletions src/KeeperData.Infrastructure/ApiClients/Fakes/FakeDataBridgeClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,17 +279,47 @@ private static DataBridgeResponse<T> GetDataBridgeResponse<T>(List<T> data, int
private List<SamCphHolding> GetSamCphHolding(string? id = null)
{
return [
new SamCphHolding {
new SamCphHolding
{
ANIMAL_PRODUCTION_USAGE_CODE = "MEAT",
ANIMAL_SPECIES_CODE = "CTT",
BATCH_ID = 1,
CHANGE_TYPE = "I",
IsDeleted = false,
UpdatedAtUtc = DateTime.UtcNow,
CreatedAtUtc = DateTime.UtcNow,
CPH = id ?? $"{_random.Next(10, 99)}{_random.Next(100, 999)}{_random.Next(1000, 9999)}",
FEATURE_NAME = Guid.NewGuid().ToString(),
COUNTRY_CODE = "GB",
CPH = id ?? $"{_random.Next(10, 99)}/{_random.Next(100, 999):000}/{_random.Next(1000, 9999)}",
CPH_RELATIONSHIP_TYPE = "MAIN",
CPH_TYPE = "PERMANENT",
CreatedAtUtc = DateTime.UtcNow,
DISEASE_TYPE = null,
EASTING = 400022,
FACILITY_BUSINSS_ACTVTY_CODE = "FACACT",
FACILITY_TYPE_CODE = "CL",
FCLTY_SUB_BSNSS_ACTVTY_CODE = "FACSUB",
FEATURE_ADDRESS_FROM_DATE = DateTime.Today.AddDays(-1),
FCLTY_SUB_BSNSS_ACTVTY_CODE = "SLG-RM-NA"
FEATURE_ADDRESS_TO_DATE = null,
FEATURE_NAME = "Feature 22",
INTERVAL = 12m,
INTERVAL_UNIT_OF_TIME = "Months",
IsDeleted = false,
LOCALITY = "Locality22",
MOVEMENT_RSTRCTN_RSN_CODE = null,
NORTHING = 500022,
OS_MAP_REFERENCE = null,
PAON_END_NUMBER = 20,
PAON_END_NUMBER_SUFFIX = 'D',
PAON_START_NUMBER = 2,
PAON_START_NUMBER_SUFFIX = 'C',
POSTCODE = "CPH22 222",
SAON_END_NUMBER = 10,
SAON_END_NUMBER_SUFFIX = 'B',
SAON_START_NUMBER = 1,
SAON_START_NUMBER_SUFFIX = 'A',
SECONDARY_CPH = "00/000/9267",
STREET = "Holding Street 22",
TOWN = "Town22",
UDPRN = "25000022",
UK_INTERNAL_CODE = "ENGLAND",
UpdatedAtUtc = DateTime.UtcNow
}];
}

Expand Down
Loading
Loading