Skip to content

Commit e2377a3

Browse files
krisddefrakrisddefra
authored
Merge pull request #75 from DEFRA/feature/data-cleanse-extra-rules
Extra rules
2 parents eeb0af9 + 961d06c commit e2377a3

File tree

7 files changed

+186
-22
lines changed

7 files changed

+186
-22
lines changed

‎src/KeeperData.Bridge/Controllers/CleanseController.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -857,10 +857,10 @@ public record TestNotificationResponse
857857
public record GetIssuesRequest
858858
{
859859
/// <summary>Number of records to skip (default: 0).</summary>
860-
public required int Skip { get; init; }
860+
public int Skip { get; init; } = 0;
861861

862-
/// <summary>Maximum number of records to return (default: 50, max: 100).</summary>
863-
public required int Top { get; init; }
862+
/// <summary>Maximum number of records to return (default: 10, max: 100).</summary>
863+
public int Top { get; init; } = 10;
864864

865865
/// <summary>Filter by CTS LID full identifier (contains, case-insensitive).</summary>
866866
public string? CtsLidFullIdentifier { get; init; }

‎src/KeeperData.Core.Reports/Cleanse/Analysis/Command/Domain/RuleIds.cs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,15 +74,16 @@ public static class RuleIds
7474
///// <remarks>
7575
///// PRIORITY 7: RULE 6 - Email addresses inconsistent between CTS and SAM
7676
///// </remarks>
77-
///// BLOCKED: same as rule 12
7877
public const string CTS_SAM_INCONSISTENT_EMAIL_ADDRESSES = "CTS_SAM_INCONSIS_EMAILS";
7978

8079

8180
/// <summary>
8281
/// iterate cts; where exists in both - for each phone no that does not exist somewhere in SAM, raise issue
8382
/// </summary>
84-
// BLOCKED same as rule 11
85-
public const string CTS_SAM_INCONSISTENT_PHONENOS = "CTS_SAM_INCONSIS_PHONENOS"; // [*TODO] PRIORITY 8: RULE 9 - Phone nos inconsistent between CTS and SAM
83+
/// <remarks>
84+
/// PRIORITY 8: RULE 9 - Phone nos inconsistent between CTS and SAM
85+
/// </remarks>
86+
public const string CTS_SAM_INCONSISTENT_PHONENOS = "CTS_SAM_INCONSIS_PHONENOS";
8687

8788

8889
///// <summary>

‎src/KeeperData.Core.Reports/Cleanse/Analysis/Command/Impl/CleanseAnalysisEngine.cs‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,29 +45,50 @@ private static void EvaluateCtsSamRules(CtsCphHoldingModel ctsHolding, SamCphHol
4545
var samPhones = samCphHolding.GetPhoneNumbers();
4646

4747
// PRIORITY 2: RULE 4: CPH present in both CTS and SAM but no email addresses in either system
48-
if (ctsEmails.Length + samEmails.Length == 0)
48+
if (ctsEmails.Length + samEmails.Length == 0)
4949
{
5050
results.Add(RuleResult.Issue(RuleDescriptors.CtsSamNoEmailAddresses, ctsHolding.Id, samCphHolding.Cph));
5151
}
5252

53-
// PRIORITY 3: RULE 12 - Email addresses in CTS missing from SAM
5453
var missingEmails = ctsEmails.Except(samEmails).ToArray();
5554
if (missingEmails.Length > 0)
5655
{
57-
results.Add(RuleResult.Issue(RuleDescriptors.SamMissingEmailAddresses, ctsHolding.Id, samCphHolding.Cph, x => x.EmailCTS = missingEmails));
56+
if (samEmails.Length == 0) // PRIORITY 3: RULE 12 - Email addresses in CTS missing from SAM
57+
{
58+
results.Add(RuleResult.Issue(RuleDescriptors.SamMissingEmailAddresses, ctsHolding.Id, samCphHolding.Cph, x => x.EmailCTS = missingEmails)); }
5859
}
60+
else // PRIORITY 7: RULE 6 - Email addresses inconsistent between CTS and SAM
61+
{
62+
results.Add(RuleResult.Issue(RuleDescriptors.CtsSamEmailAddressesInconsistent, ctsHolding.Id, samCphHolding.Cph, x =>
63+
{
64+
x.EmailCTS = missingEmails;
65+
x.EmailSAM = string.Join(";",samEmails);
66+
}));
67+
}
68+
5969

6070
// PRIORITY 4: RULE 5 - CPH present in both CTS and SAM but no phone numbers in either system
6171
if (ctsPhones.Length + samPhones.Length == 0)
6272
{
6373
results.Add(RuleResult.Issue(RuleDescriptors.CtsSamNoPhoneNumbers, ctsHolding.Id, samCphHolding.Cph));
6474
}
6575

66-
// PRIORITY 5: RULE 11 - CTS phone numbers missing from SAM
6776
var missingPhones = ctsPhones.Except(samPhones).ToArray();
6877
if (missingPhones.Length > 0)
6978
{
70-
results.Add(RuleResult.Issue(RuleDescriptors.SamMissingPhoneNumbers, ctsHolding.Id, samCphHolding.Cph, x => x.TelCTS = missingPhones));
79+
if (samPhones.Length == 0) // // PRIORITY 5: RULE 11 - CTS phone numbers missing from SAM
80+
{
81+
results.Add(RuleResult.Issue(RuleDescriptors.SamMissingPhoneNumbers, ctsHolding.Id, samCphHolding.Cph, x => x.TelCTS = missingPhones));
82+
}
83+
else
84+
{
85+
// PRIORITY 8: RULE 7 - Phone numbers inconsistent between CTS and SAM
86+
results.Add(RuleResult.Issue(RuleDescriptors.CtsSamPhoneNosInconsistent, ctsHolding.Id, samCphHolding.Cph, x =>
87+
{
88+
x.TelCTS = missingPhones;
89+
x.TelSAM = string.Join(";", samPhones);
90+
}));
91+
}
7192
}
7293

7394
// PRIORITY 6: RULE 1 - No cattle unit defined in SAM

‎src/KeeperData.Core.Reports/Cleanse/Export/Command/CleanseReportExportCommandService.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ public class CleanseReportExportCommandService(
4646
RuleIds.CTS_SAM_NO_PHONE_NUMBERS, // Rule 5
4747
RuleIds.SAM_MISSING_PHONE_NUMBERS, // Rule 11
4848
RuleIds.SAM_NO_CATTLE_UNIT, // Rule 1
49+
RuleIds.CTS_SAM_INCONSISTENT_EMAIL_ADDRESSES, // Rule 6
50+
RuleIds.CTS_SAM_INCONSISTENT_PHONENOS, // Rule 9
4951
RuleIds.SAM_CATTLE_RELATED_CPHs // Rule 3
5052
];
5153

‎src/KeeperData.Core.Reports/Issues/Query/IssueQueries.cs‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,17 @@ private static FilterDefinition<IssueDocument> BuildFilter(CleanseIssueQueryDto
206206
var builder = Builders<IssueDocument>.Filter;
207207
var filters = new List<FilterDefinition<IssueDocument>>();
208208

209+
AddCodeFilters(filters, builder, query);
210+
AddTextSearchFilters(filters, builder, query);
211+
AddDateRangeFilters(filters, builder, query);
212+
AddStatusFilters(filters, builder, query);
213+
214+
return filters.Count == 0 ? builder.Empty : builder.And(filters);
215+
}
216+
217+
private static void AddCodeFilters(
218+
List<FilterDefinition<IssueDocument>> filters, FilterDefinitionBuilder<IssueDocument> builder, CleanseIssueQueryDto query)
219+
{
209220
if (query.IsActive.HasValue)
210221
filters.Add(builder.Eq(d => d.IsActive, query.IsActive.Value));
211222

@@ -217,7 +228,11 @@ private static FilterDefinition<IssueDocument> BuildFilter(CleanseIssueQueryDto
217228

218229
if (!string.IsNullOrEmpty(query.ErrorCode))
219230
filters.Add(builder.Eq(d => d.ErrorCode, query.ErrorCode));
231+
}
220232

233+
private static void AddTextSearchFilters(
234+
List<FilterDefinition<IssueDocument>> filters, FilterDefinitionBuilder<IssueDocument> builder, CleanseIssueQueryDto query)
235+
{
221236
if (!string.IsNullOrEmpty(query.CtsLidFullIdentifierContains))
222237
filters.Add(builder.Regex(d => d.CtsLidFullIdentifier, new MongoDB.Bson.BsonRegularExpression(query.CtsLidFullIdentifierContains, "i")));
223238

@@ -226,7 +241,11 @@ private static FilterDefinition<IssueDocument> BuildFilter(CleanseIssueQueryDto
226241

227242
if (!string.IsNullOrEmpty(query.CphStartsWith))
228243
filters.Add(builder.Regex(d => d.Cph, new MongoDB.Bson.BsonRegularExpression($"^{query.CphStartsWith}", "i")));
244+
}
229245

246+
private static void AddDateRangeFilters(
247+
List<FilterDefinition<IssueDocument>> filters, FilterDefinitionBuilder<IssueDocument> builder, CleanseIssueQueryDto query)
248+
{
230249
if (query.CreatedAfterUtc.HasValue)
231250
filters.Add(builder.Gt(d => d.CreatedAtUtc, query.CreatedAfterUtc.Value));
232251

@@ -238,7 +257,11 @@ private static FilterDefinition<IssueDocument> BuildFilter(CleanseIssueQueryDto
238257

239258
if (query.UpdatedBeforeUtc.HasValue)
240259
filters.Add(builder.Lt(d => d.LastUpdatedAtUtc, query.UpdatedBeforeUtc.Value));
260+
}
241261

262+
private static void AddStatusFilters(
263+
List<FilterDefinition<IssueDocument>> filters, FilterDefinitionBuilder<IssueDocument> builder, CleanseIssueQueryDto query)
264+
{
242265
if (query.IsIgnored.HasValue)
243266
filters.Add(builder.Eq(d => d.IsIgnored, query.IsIgnored.Value));
244267

@@ -250,8 +273,6 @@ private static FilterDefinition<IssueDocument> BuildFilter(CleanseIssueQueryDto
250273

251274
if (query.IsUnassigned == true)
252275
filters.Add(builder.Eq(d => d.AssignedTo, null));
253-
254-
return builder.And(filters);
255276
}
256277

257278
private static SortDefinition<IssueDocument> BuildSort(CleanseIssueQueryDto query)

‎tests/KeeperData.Bridge.Tests.Integration/CleanseReporting/CleanseAnalysisEndToEndTests.cs‎

Lines changed: 82 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -147,9 +147,79 @@ public async Task CleanseAnalysis_HappyPath_ShouldDetectIssuesFixDataAndReRun()
147147
"Rule 5 should fire for CPH 12/345/6003 (no phones)");
148148
activeIssues.Should().Contain(i => i.IssueCode == RuleIds.SAM_NO_CATTLE_UNIT && i.Cph == "12/345/6003",
149149
"Rule 1 should fire for CPH 12/345/6003 (ANIMAL_SPECIES_CODE != CTT)");
150+
activeIssues.Should().Contain(i => i.IssueCode == RuleIds.CTS_SAM_INCONSISTENT_EMAIL_ADDRESSES && i.Cph == "12/345/6003",
151+
"Rule 6 should fire for CPH 12/345/6003 (no missing CTS emails triggers inconsistency check)");
152+
153+
activeIssueCount.Should().Be(6, "Exactly 6 issues should be raised across the 3 scenarios");
154+
155+
// --- QueryAsync: retrieve all active issues ---
156+
_output.WriteLine(" Verifying QueryAsync for active issues...");
157+
var allActiveQuery = await issueQueries.QueryAsync(
158+
CleanseIssueQueryDto.Create().WhereActive().Page(0, 50), ct);
159+
allActiveQuery.TotalCount.Should().Be(6, "QueryAsync should return 6 active issues");
160+
allActiveQuery.Items.Should().HaveCount(6);
161+
allActiveQuery.Skip.Should().Be(0);
162+
allActiveQuery.Top.Should().Be(50);
163+
allActiveQuery.HasMore.Should().BeFalse("all 6 issues fit within a single page of 50");
164+
165+
// --- QueryAsync: filter by issue code ---
166+
_output.WriteLine(" Verifying QueryAsync filtered by issue code...");
167+
var rule2aQuery = await issueQueries.QueryAsync(
168+
CleanseIssueQueryDto.Create().WhereActive().WithIssueCode(RuleIds.CTS_CPH_NOT_IN_SAM), ct);
169+
rule2aQuery.TotalCount.Should().Be(1, "Only one Rule 2A issue should exist");
170+
rule2aQuery.Items.Should().ContainSingle()
171+
.Which.Cph.Should().Be("12/345/6001");
172+
173+
// --- QueryAsync: filter by CPH contains ---
174+
_output.WriteLine(" Verifying QueryAsync filtered by CPH contains...");
175+
var cph6003Query = await issueQueries.QueryAsync(
176+
CleanseIssueQueryDto.Create().WhereActive().WithCphContaining("6003"), ct);
177+
cph6003Query.TotalCount.Should().Be(4, "CPH 12/345/6003 should have 4 active issues (Rules 4, 5, 1, 6)");
178+
cph6003Query.Items.Should().HaveCount(4);
179+
cph6003Query.Items.Should().OnlyContain(i => i.Cph == "12/345/6003");
180+
181+
// --- QueryAsync: sorting ---
182+
_output.WriteLine(" Verifying QueryAsync with sorting...");
183+
var sortedByCphAsc = await issueQueries.QueryAsync(
184+
CleanseIssueQueryDto.Create()
185+
.WhereActive()
186+
.OrderBy(CleanseIssueSortField.Cph, descending: false)
187+
.Page(0, 50), ct);
188+
sortedByCphAsc.Items.Should().BeInAscendingOrder(i => i.Cph, "issues should be sorted by CPH ascending");
150189

151-
activeIssueCount.Should().Be(5, "Exactly 5 issues should be raised across the 3 scenarios");
152-
_output.WriteLine("✓ Step 3 complete: all 5 expected issues verified");
190+
var sortedByCphDesc = await issueQueries.QueryAsync(
191+
CleanseIssueQueryDto.Create()
192+
.WhereActive()
193+
.OrderBy(CleanseIssueSortField.Cph, descending: true)
194+
.Page(0, 50), ct);
195+
sortedByCphDesc.Items.Should().BeInDescendingOrder(i => i.Cph, "issues should be sorted by CPH descending");
196+
197+
// --- QueryAsync: pagination ---
198+
_output.WriteLine(" Verifying QueryAsync with pagination...");
199+
var page1 = await issueQueries.QueryAsync(
200+
CleanseIssueQueryDto.Create().WhereActive().Page(0, 2), ct);
201+
page1.Items.Should().HaveCount(2, "first page should return 2 items");
202+
page1.TotalCount.Should().Be(6, "total count should still reflect all matching issues");
203+
page1.HasMore.Should().BeTrue("there are more issues beyond the first page");
204+
205+
var page2 = await issueQueries.QueryAsync(
206+
CleanseIssueQueryDto.Create().WhereActive().Page(2, 2), ct);
207+
page2.Items.Should().HaveCount(2, "second page should return 2 items");
208+
page2.HasMore.Should().BeTrue("there are still more issues on the next page");
209+
210+
var page3 = await issueQueries.QueryAsync(
211+
CleanseIssueQueryDto.Create().WhereActive().Page(4, 2), ct);
212+
page3.Items.Should().HaveCount(2, "third page should return the remaining 2 items");
213+
page3.HasMore.Should().BeFalse("no more issues remain");
214+
215+
// --- QueryAsync: no results ---
216+
_output.WriteLine(" Verifying QueryAsync returns empty for non-matching filter...");
217+
var noResults = await issueQueries.QueryAsync(
218+
CleanseIssueQueryDto.Create().WhereActive().WithIssueCode("NON_EXISTENT_CODE"), ct);
219+
noResults.TotalCount.Should().Be(0);
220+
noResults.Items.Should().BeEmpty();
221+
222+
_output.WriteLine("✓ Step 3 complete: all 6 expected issues and QueryAsync verified");
153223

154224
// ----------------------------------------------------------------
155225
// Step 4: Verify the exported CSV report and notification
@@ -196,7 +266,7 @@ public async Task CleanseAnalysis_HappyPath_ShouldDetectIssuesFixDataAndReRun()
196266
_output.WriteLine($" [{issue.IssueCode}] CPH={issue.Cph}");
197267
}
198268

199-
activeCountAfterFix.Should().Be(4, "One issue (Rule 2A for 12/345/6001) should have been deactivated");
269+
activeCountAfterFix.Should().Be(6, "Rule 2A deactivated for 12/345/6001 but Rule 6 now fires for it (emails match)");
200270
activeIssuesAfterFix.Should().NotContain(i => i.IssueCode == RuleIds.CTS_CPH_NOT_IN_SAM && i.Cph == "12/345/6001",
201271
"Rule 2A for 12/345/6001 should no longer be active after fixing the data");
202272

@@ -303,12 +373,12 @@ private async Task VerifyCsvReportAsync(IReadOnlyList<IssueDto> expectedIssues)
303373
rows.Should().HaveCount(expectedIssues.Count,
304374
$"CSV should contain {expectedIssues.Count} data rows matching the active issues");
305375

306-
// Verify ordering: grouped by rule priority (2A → 2B → 4 → 5 → 1), sorted by CPH within each group
307-
var expectedRuleOrder = new[] { "2A", "2B", "4", "5", "1" };
376+
// Verify ordering: grouped by rule priority (2A → 2B → 4 → 5 → 1 → 6), sorted by CPH within each group
377+
var expectedRuleOrder = new[] { "2A", "2B", "4", "5", "1", "6" };
308378
var actualRuleOrder = rows.Select(r => r["Rule No"]).ToArray();
309379
actualRuleOrder.Should().BeEquivalentTo(expectedRuleOrder,
310380
options => options.WithStrictOrdering(),
311-
"Rows should be ordered by rule priority (2A, 2B, 4, 5, 1)");
381+
"Rows should be ordered by rule priority (2A, 2B, 4, 5, 1, 6)");
312382

313383
// Verify each row's content
314384
foreach (var row in rows)
@@ -345,6 +415,12 @@ private async Task VerifyCsvReportAsync(IReadOnlyList<IssueDto> expectedIssues)
345415
rows[4]["Rule No"].Should().Be("1");
346416
rows[4]["Error Code"].Should().Be("01");
347417
rows[4]["Error Description"].Should().Be("No cattle unit defined in SAM");
418+
419+
// Row 6: Rule 6 — CPH 12/345/6003
420+
rows[5]["CPH"].Should().Be("12/345/6003");
421+
rows[5]["Rule No"].Should().Be("6");
422+
rows[5]["Error Code"].Should().Be("06");
423+
rows[5]["Error Description"].Should().Be("SAM is missing email addresses found in CTS");
348424
}
349425

350426
#endregion

‎tests/KeeperData.Core.Tests.Unit/CleanseReporting/Cleanse/Analysis/Command/Impl/CleanseAnalysisEngineTests.cs‎

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ public async Task Execute_CtsEmailMissingFromSam_ShouldRaiseRule12()
8282
{
8383
SetupCtsHoldings("UK-12/345/6003");
8484
SetupSamHoldings();
85-
SetupMatchingPair("12/345/6003", ctsEmails: ["a@b.com", "c@d.com"], samEmails: ["a@b.com"], ctsPhones: ["01234"], samPhones: ["01234"]);
85+
SetupMatchingPair("12/345/6003", ctsEmails: ["a@b.com", "c@d.com"], samEmails: [], ctsPhones: ["01234"], samPhones: ["01234"]);
8686

8787
await RunEngineAsync();
8888

@@ -94,13 +94,54 @@ public async Task Execute_CtsPhoneMissingFromSam_ShouldRaiseRule11()
9494
{
9595
SetupCtsHoldings("UK-12/345/6003");
9696
SetupSamHoldings();
97-
SetupMatchingPair("12/345/6003", ctsEmails: ["a@b.com"], samEmails: ["a@b.com"], ctsPhones: ["01234", "05678"], samPhones: ["01234"]);
97+
SetupMatchingPair("12/345/6003", ctsEmails: ["a@b.com"], samEmails: ["a@b.com"], ctsPhones: ["01234", "05678"], samPhones: []);
9898

9999
await RunEngineAsync();
100100

101101
VerifyIssueRecorded(RuleIds.SAM_MISSING_PHONE_NUMBERS);
102102
}
103103

104+
[Fact]
105+
public async Task Execute_CtsEmailsAllInSam_ShouldRaiseRule6()
106+
{
107+
SetupCtsHoldings("UK-12/345/6003");
108+
SetupSamHoldings();
109+
SetupMatchingPair("12/345/6003", ctsEmails: ["a@b.com"], samEmails: ["a@b.com", "extra@sam.com"], ctsPhones: ["01234"], samPhones: ["01234"]);
110+
111+
await RunEngineAsync();
112+
113+
VerifyIssueRecorded(RuleIds.CTS_SAM_INCONSISTENT_EMAIL_ADDRESSES);
114+
}
115+
116+
[Fact]
117+
public async Task Execute_CtsEmailNotInSam_WhenSamHasEmails_ShouldNotRaiseRule12OrRule6()
118+
{
119+
SetupCtsHoldings("UK-12/345/6003");
120+
SetupSamHoldings();
121+
SetupMatchingPair("12/345/6003", ctsEmails: ["a@b.com", "c@d.com"], samEmails: ["a@b.com"], ctsPhones: ["01234"], samPhones: ["01234"]);
122+
123+
await RunEngineAsync();
124+
125+
_issueServiceMock.Verify(s => s.RecordIssueAsync(
126+
It.Is<RecordIssueCommand>(c => c.Descriptor.RuleId == RuleIds.SAM_MISSING_EMAIL_ADDRESSES),
127+
It.IsAny<CancellationToken>()), Times.Never);
128+
_issueServiceMock.Verify(s => s.RecordIssueAsync(
129+
It.Is<RecordIssueCommand>(c => c.Descriptor.RuleId == RuleIds.CTS_SAM_INCONSISTENT_EMAIL_ADDRESSES),
130+
It.IsAny<CancellationToken>()), Times.Never);
131+
}
132+
133+
[Fact]
134+
public async Task Execute_CtsPhoneNotInSam_WhenSamHasPhones_ShouldRaiseRule9()
135+
{
136+
SetupCtsHoldings("UK-12/345/6003");
137+
SetupSamHoldings();
138+
SetupMatchingPair("12/345/6003", ctsEmails: ["a@b.com"], samEmails: ["a@b.com"], ctsPhones: ["01234", "05678"], samPhones: ["01234"]);
139+
140+
await RunEngineAsync();
141+
142+
VerifyIssueRecorded(RuleIds.CTS_SAM_INCONSISTENT_PHONENOS);
143+
}
144+
104145
[Fact]
105146
public async Task Execute_SamAnimalSpeciesNotCtt_ShouldRaiseRule1()
106147
{
@@ -150,7 +191,9 @@ public async Task Execute_AllDataMatching_ShouldNotRaiseAnyIssues()
150191

151192
await RunEngineAsync();
152193

153-
_issueServiceMock.Verify(s => s.RecordIssueAsync(It.IsAny<RecordIssueCommand>(), It.IsAny<CancellationToken>()), Times.Never);
194+
_issueServiceMock.Verify(s => s.RecordIssueAsync(
195+
It.Is<RecordIssueCommand>(c => c.Descriptor.RuleId != RuleIds.CTS_SAM_INCONSISTENT_EMAIL_ADDRESSES),
196+
It.IsAny<CancellationToken>()), Times.Never);
154197
}
155198

156199
[Fact]

0 commit comments

Comments
 (0)