Skip to content

Commit 01f07ec

Browse files
ntsimntsim
committed
EES-5437 Fix deleted location levels not being used in location mapping updates
This fixes a bug in location mappings where any deleted location levels would not be considered during mapping updates for: - The `LocationMappingsComplete` flag - The calculated version number These are now correctly handled meaning that deleted location levels result in major updates, and their options are not considered when calculating the `LocationMappingsComplete` flag.
1 parent 1a929d0 commit 01f07ec

File tree

2 files changed

+192
-70
lines changed

2 files changed

+192
-70
lines changed

‎src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Controllers/Api/Public.Data/DataSetVersionMappingControllerTests.cs

+141-47
Original file line numberDiff line numberDiff line change
@@ -393,18 +393,19 @@ originalCountryMappingToUpdate with
393393
}
394394

395395
[Theory]
396-
[InlineData(MappingType.ManualMapped, MappingType.AutoNone, false)]
397-
[InlineData(MappingType.ManualMapped, MappingType.AutoMapped, true)]
398-
[InlineData(MappingType.ManualMapped, MappingType.ManualMapped, true)]
399-
[InlineData(MappingType.ManualMapped, MappingType.ManualNone, true)]
400-
[InlineData(MappingType.ManualNone, MappingType.AutoNone, false)]
401-
[InlineData(MappingType.ManualNone, MappingType.AutoMapped, true)]
402-
[InlineData(MappingType.ManualNone, MappingType.ManualMapped, true)]
403-
[InlineData(MappingType.ManualNone, MappingType.ManualNone, true)]
404-
public async Task Success_MappingsComplete(
396+
[InlineData(MappingType.ManualMapped, MappingType.AutoNone, false, "2.0.0")]
397+
[InlineData(MappingType.ManualMapped, MappingType.AutoMapped, true, "1.1.0")]
398+
[InlineData(MappingType.ManualMapped, MappingType.ManualMapped, true, "1.1.0")]
399+
[InlineData(MappingType.ManualMapped, MappingType.ManualNone, true, "2.0.0")]
400+
[InlineData(MappingType.ManualNone, MappingType.AutoNone, false, "2.0.0")]
401+
[InlineData(MappingType.ManualNone, MappingType.AutoMapped, true, "2.0.0")]
402+
[InlineData(MappingType.ManualNone, MappingType.ManualMapped, true, "2.0.0")]
403+
[InlineData(MappingType.ManualNone, MappingType.ManualNone, true, "2.0.0")]
404+
public async Task Success_MappingsCompleteAndVersionUpdated(
405405
MappingType updatedMappingType,
406406
MappingType unchangedMappingType,
407-
bool expectedMappingsComplete)
407+
bool expectedMappingsComplete,
408+
string expectedVersion)
408409
{
409410
DataSet dataSet = DataFixture
410411
.DefaultDataSet()
@@ -484,18 +485,16 @@ await TestApp.AddTestData<PublicDataDbContext>(context =>
484485

485486
await TestApp.AddTestData<ContentDbContext>(context => context.ReleaseFiles.Add(releaseFile));
486487

487-
var mappingCandidateKey = updatedMappingType == MappingType.ManualMapped
488-
? "target-location-1-key"
489-
: null;
490-
491488
List<LocationMappingUpdateRequest> updates =
492489
[
493490
new()
494491
{
495492
Level = GeographicLevel.LocalAuthority,
496493
SourceKey = "source-location-1-key",
497494
Type = updatedMappingType,
498-
CandidateKey = mappingCandidateKey
495+
CandidateKey = updatedMappingType == MappingType.ManualMapped
496+
? "target-location-1-key"
497+
: null
499498
}
500499
];
501500

@@ -507,26 +506,22 @@ await TestApp.AddTestData<PublicDataDbContext>(context =>
507506

508507
var updatedMappings = await TestApp.GetDbContext<PublicDataDbContext>()
509508
.DataSetVersionMappings
509+
.Include(m => m.TargetDataSetVersion)
510510
.SingleAsync(m => m.TargetDataSetVersionId == nextDataSetVersion.Id);
511511

512-
// Assert that the batch save calculates the LocationMappingsComplete flag as expected given
513-
// the combination of the requested mapping update and the existing mapping that is untouched.
514512
Assert.Equal(expectedMappingsComplete, updatedMappings.LocationMappingsComplete);
513+
514+
Assert.Equal(expectedVersion, updatedMappings.TargetDataSetVersion.SemVersion());
515+
516+
var updatedReleaseFile = await TestApp.GetDbContext<ContentDbContext>()
517+
.ReleaseFiles
518+
.SingleAsync(rf => rf.PublicApiDataSetId == updatedMappings.TargetDataSetVersion.DataSetId);
519+
520+
Assert.Equal(expectedVersion, updatedReleaseFile.PublicApiDataSetVersion);
515521
}
516522

517-
[Theory]
518-
[InlineData(MappingType.ManualMapped, MappingType.AutoNone, "2.0.0")]
519-
[InlineData(MappingType.ManualMapped, MappingType.AutoMapped, "1.1.0")]
520-
[InlineData(MappingType.ManualMapped, MappingType.ManualMapped, "1.1.0")]
521-
[InlineData(MappingType.ManualMapped, MappingType.ManualNone, "2.0.0")]
522-
[InlineData(MappingType.ManualNone, MappingType.AutoNone, "2.0.0")]
523-
[InlineData(MappingType.ManualNone, MappingType.AutoMapped, "2.0.0")]
524-
[InlineData(MappingType.ManualNone, MappingType.ManualMapped, "2.0.0")]
525-
[InlineData(MappingType.ManualNone, MappingType.ManualNone, "2.0.0")]
526-
public async Task Success_VersionUpdate(
527-
MappingType updatedMappingType,
528-
MappingType unchangedMappingType,
529-
string expectedVersion)
523+
[Fact]
524+
public async Task Success_DeletedLevel_MajorUpdate()
530525
{
531526
DataSet dataSet = DataFixture
532527
.DefaultDataSet()
@@ -573,6 +568,7 @@ await TestApp.AddTestData<PublicDataDbContext>(context =>
573568
.AddCandidate(
574569
targetKey: "target-location-1-key",
575570
candidate: DataFixture.DefaultMappableLocationOption()))
571+
// Country level has been deleted and has no candidates.
576572
.AddLevel(
577573
level: GeographicLevel.Country,
578574
mappings: DataFixture
@@ -582,15 +578,114 @@ await TestApp.AddTestData<PublicDataDbContext>(context =>
582578
mapping: DataFixture
583579
.DefaultLocationOptionMapping()
584580
.WithSource(DataFixture.DefaultMappableLocationOption())
585-
.WithType(unchangedMappingType)
586-
.WithCandidateKey(unchangedMappingType switch
587-
{
588-
MappingType.ManualMapped or MappingType.AutoMapped => "target-location-1-key",
589-
_ => null
590-
}))
581+
.WithAutoNone())));
582+
583+
await TestApp.AddTestData<PublicDataDbContext>(context =>
584+
{
585+
context.DataSetVersionMappings.Add(mappings);
586+
});
587+
588+
ReleaseFile releaseFile = DataFixture.DefaultReleaseFile()
589+
.WithId(nextDataSetVersion.Release.ReleaseFileId)
590+
.WithReleaseVersion(DataFixture.DefaultReleaseVersion())
591+
.WithFile(DataFixture.DefaultFile())
592+
.WithPublicApiDataSetId(nextDataSetVersion.DataSetId)
593+
.WithPublicApiDataSetVersion(nextDataSetVersion.SemVersion());
594+
595+
await TestApp.AddTestData<ContentDbContext>(context => context.ReleaseFiles.Add(releaseFile));
596+
597+
List<LocationMappingUpdateRequest> updates =
598+
[
599+
new()
600+
{
601+
Level = GeographicLevel.LocalAuthority,
602+
SourceKey = "source-location-1-key",
603+
Type = MappingType.ManualMapped,
604+
CandidateKey = "target-location-1-key"
605+
}
606+
];
607+
608+
var response = await ApplyBatchLocationMappingUpdates(
609+
nextDataSetVersionId: nextDataSetVersion.Id,
610+
updates: updates);
611+
612+
response.AssertOk<BatchLocationMappingUpdatesResponseViewModel>();
613+
614+
var updatedMappings = await TestApp.GetDbContext<PublicDataDbContext>()
615+
.DataSetVersionMappings
616+
.Include(m => m.TargetDataSetVersion)
617+
.SingleAsync(m => m.TargetDataSetVersionId == nextDataSetVersion.Id);
618+
619+
// This update completes the mapping but as there's a
620+
// location level deletion, it's a major version update.
621+
Assert.True(updatedMappings.LocationMappingsComplete);
622+
623+
Assert.Equal("2.0.0", updatedMappings.TargetDataSetVersion.SemVersion());
624+
625+
var updatedReleaseFile = await TestApp.GetDbContext<ContentDbContext>()
626+
.ReleaseFiles
627+
.SingleAsync(rf => rf.PublicApiDataSetId == updatedMappings.TargetDataSetVersion.DataSetId);
628+
629+
Assert.Equal("2.0.0", updatedReleaseFile.PublicApiDataSetVersion);
630+
}
631+
632+
[Fact]
633+
public async Task Success_AddedLevel_MinorUpdate()
634+
{
635+
DataSet dataSet = DataFixture
636+
.DefaultDataSet()
637+
.WithStatusPublished();
638+
639+
await TestApp.AddTestData<PublicDataDbContext>(context => context.DataSets.Add(dataSet));
640+
641+
DataSetVersion currentDataSetVersion = DataFixture
642+
.DefaultDataSetVersion(filters: 1, indicators: 1, locations: 1, timePeriods: 2)
643+
.WithVersionNumber(major: 1, minor: 0)
644+
.WithStatusPublished()
645+
.WithDataSet(dataSet)
646+
.FinishWith(dsv => dsv.DataSet.LatestLiveVersion = dsv);
647+
648+
DataSetVersion nextDataSetVersion = DataFixture
649+
.DefaultDataSetVersion(filters: 1, indicators: 1, locations: 1, timePeriods: 2)
650+
.WithVersionNumber(major: 1, minor: 1)
651+
.WithStatusDraft()
652+
.WithDataSet(dataSet)
653+
.FinishWith(dsv => dsv.DataSet.LatestDraftVersion = dsv);
654+
655+
await TestApp.AddTestData<PublicDataDbContext>(context =>
656+
{
657+
context.DataSetVersions.AddRange(currentDataSetVersion, nextDataSetVersion);
658+
context.DataSets.Update(dataSet);
659+
});
660+
661+
DataSetVersionMapping mappings = DataFixture
662+
.DefaultDataSetVersionMapping()
663+
.WithSourceDataSetVersionId(currentDataSetVersion.Id)
664+
.WithTargetDataSetVersionId(nextDataSetVersion.Id)
665+
.WithLocationMappingPlan(DataFixture
666+
.DefaultLocationMappingPlan()
667+
.AddLevel(
668+
level: GeographicLevel.LocalAuthority,
669+
mappings: DataFixture
670+
.DefaultLocationLevelMappings()
671+
.AddMapping(
672+
sourceKey: "source-location-1-key",
673+
mapping: DataFixture
674+
.DefaultLocationOptionMapping()
675+
.WithSource(DataFixture.DefaultMappableLocationOption())
676+
.WithNoMapping())
591677
.AddCandidate(
592678
targetKey: "target-location-1-key",
593-
candidate: DataFixture.DefaultMappableLocationOption())));
679+
candidate: DataFixture.DefaultMappableLocationOption()))
680+
// Country level has been added and only has candidates.
681+
.AddLevel(
682+
level: GeographicLevel.Country,
683+
mappings: DataFixture
684+
.DefaultLocationLevelMappings()
685+
.AddCandidate(
686+
targetKey: "source-location-1-key",
687+
candidate: DataFixture
688+
.DefaultMappableLocationOption())));
594689

595690
await TestApp.AddTestData<PublicDataDbContext>(context =>
596691
{
@@ -600,24 +695,20 @@ await TestApp.AddTestData<PublicDataDbContext>(context =>
600695
ReleaseFile releaseFile = DataFixture.DefaultReleaseFile()
601696
.WithId(nextDataSetVersion.Release.ReleaseFileId)
602697
.WithReleaseVersion(DataFixture.DefaultReleaseVersion())
603-
.WithFile(DataFixture.DefaultFile(FileType.Data))
698+
.WithFile(DataFixture.DefaultFile())
604699
.WithPublicApiDataSetId(nextDataSetVersion.DataSetId)
605700
.WithPublicApiDataSetVersion(nextDataSetVersion.SemVersion());
606701

607702
await TestApp.AddTestData<ContentDbContext>(context => context.ReleaseFiles.Add(releaseFile));
608703

609-
var mappingCandidateKey = updatedMappingType == MappingType.ManualMapped
610-
? "target-location-1-key"
611-
: null;
612-
613704
List<LocationMappingUpdateRequest> updates =
614705
[
615706
new()
616707
{
617708
Level = GeographicLevel.LocalAuthority,
618709
SourceKey = "source-location-1-key",
619-
Type = updatedMappingType,
620-
CandidateKey = mappingCandidateKey
710+
Type = MappingType.ManualMapped,
711+
CandidateKey = "target-location-1-key"
621712
}
622713
];
623714

@@ -632,14 +723,17 @@ await TestApp.AddTestData<PublicDataDbContext>(context =>
632723
.Include(m => m.TargetDataSetVersion)
633724
.SingleAsync(m => m.TargetDataSetVersionId == nextDataSetVersion.Id);
634725

635-
// Assert that the batch save calculates the next data set version correctly.
636-
Assert.Equal(expectedVersion, updatedMappings.TargetDataSetVersion.SemVersion());
726+
// This update completes the mapping as a location level was added
727+
// and isn't considered as needing to be mapped - minor version update.
728+
Assert.True(updatedMappings.LocationMappingsComplete);
729+
730+
Assert.Equal("1.1.0", updatedMappings.TargetDataSetVersion.SemVersion());
637731

638732
var updatedReleaseFile = await TestApp.GetDbContext<ContentDbContext>()
639733
.ReleaseFiles
640734
.SingleAsync(rf => rf.PublicApiDataSetId == updatedMappings.TargetDataSetVersion.DataSetId);
641735

642-
Assert.Equal(expectedVersion, updatedReleaseFile.PublicApiDataSetVersion);
736+
Assert.Equal("1.1.0", updatedReleaseFile.PublicApiDataSetVersion);
643737
}
644738

645739
[Fact]

0 commit comments

Comments
 (0)