From 63e714e638a99917e6455b15a1b45ce137ec45a4 Mon Sep 17 00:00:00 2001 From: Mark Youngman Date: Fri, 13 Dec 2024 16:35:11 +0000 Subject: [PATCH] EES-5738 Add tests --- .../DataSetFilesControllerTests.cs | 47 +++++++++++++++++ .../Fixtures/DataImportGeneratorExtensions.cs | 34 ++++++++++++- ...tFileGeographicLevelGeneratorExtensions.cs | 50 +++++++++++++++++++ .../Fixtures/FileGeneratorExtensions.cs | 23 ++++++++- .../File.cs | 2 +- .../DataSetFileService.cs | 4 +- .../Functions/ProcessorStage3Tests.cs | 21 ++++---- .../Services/DataImportServiceTests.cs | 1 + .../Services/DataImportService.cs | 1 + .../data-catalogue/__data__/testDataSets.ts | 2 +- .../__tests__/DataCataloguePage.test.tsx | 26 ++++++++++ .../__tests__/DataSetSummary.test.tsx | 4 +- 12 files changed, 195 insertions(+), 20 deletions(-) create mode 100644 src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/DataSetFileGeographicLevelGeneratorExtensions.cs diff --git a/src/GovUk.Education.ExploreEducationStatistics.Content.Api.Tests/Controllers/DataSetFilesControllerTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Content.Api.Tests/Controllers/DataSetFilesControllerTests.cs index 91a6b434769..b69ddb03ce6 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.Api.Tests/Controllers/DataSetFilesControllerTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Api.Tests/Controllers/DataSetFilesControllerTests.cs @@ -95,6 +95,51 @@ await TestApp.AddTestData(context => AssertResultsForExpectedReleaseFiles(publication1Release1Version1Files, pagedResult.Results); } + [Fact] + public async Task FilterByGeographicLevel_Success() + { + var (publication1, publication2) = _fixture + .DefaultPublication() + // Publications each have a published release version + .WithReleases(_fixture.DefaultRelease(publishedVersions: 1) + .Generate(1)) + .WithTheme(_fixture.DefaultTheme()) + .GenerateTuple2(); + + var publication1Release1Version1Files = _fixture.DefaultReleaseFile() + .WithReleaseVersion(publication1.ReleaseVersions[0]) + .WithFiles(_fixture.DefaultFile(FileType.Data) + .WithDataSetFileMeta(_fixture.DefaultDataSetFileMeta() + .WithGeographicLevels( + [GeographicLevel.Country, GeographicLevel.LocalAuthority, GeographicLevel.Institution])) + .WithDataSetFileGeographicLevels( + [GeographicLevel.Country, GeographicLevel.LocalAuthority, GeographicLevel.Institution]) + .GenerateList(1)) + .GenerateList(); + + var publication2Release1Version1Files = GenerateDataSetFilesForReleaseVersion(publication2.ReleaseVersions[0]); + + await TestApp.AddTestData(context => + { + context.ReleaseFiles.AddRange(publication1Release1Version1Files); + context.ReleaseFiles.AddRange(publication2Release1Version1Files); + }); + + MemoryCacheService + .SetupNotFoundForAnyKey>(); + + var query = new DataSetFileListRequest(GeographicLevel: GeographicLevel.Institution.GetEnumValue()); + var response = await ListDataSetFiles(query); + + MockUtils.VerifyAllMocks(MemoryCacheService); + + var pagedResult = response.AssertOk>(); + + pagedResult.AssertHasExpectedPagingAndResultCount( + expectedTotalResults: 1); + AssertResultsForExpectedReleaseFiles(publication1Release1Version1Files, pagedResult.Results); + } + [Fact] public async Task FilterByPublicationId_Success() { @@ -1675,6 +1720,7 @@ private async Task ListDataSetFiles( { "themeId", request.ThemeId?.ToString() }, { "publicationId", request.PublicationId?.ToString() }, { "releaseId", request.ReleaseId?.ToString() }, + { "geographicLevel", request.GeographicLevel }, { "latestOnly", request.LatestOnly?.ToString() }, { "searchTerm", request.SearchTerm }, { "sort", request.Sort?.ToString() }, @@ -1737,6 +1783,7 @@ private List GenerateDataSetFilesForReleaseVersion( .WithReleaseVersion(releaseVersion) .WithFiles(_fixture.DefaultFile(FileType.Data) .WithDataSetFileMeta(_fixture.DefaultDataSetFileMeta()) + .WithDataSetFileGeographicLevels([GeographicLevel.Country]) .GenerateList(numberOfDataSets)) .GenerateList(); } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/DataImportGeneratorExtensions.cs b/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/DataImportGeneratorExtensions.cs index 7a02d7410cf..0eab870e422 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/DataImportGeneratorExtensions.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/DataImportGeneratorExtensions.cs @@ -19,8 +19,15 @@ public static Generator WithSubjectId( public static Generator WithDefaultFiles( this Generator generator, - string dataFileName) - => generator.ForInstance(s => s.SetDefaultFiles(dataFileName)); + string dataFileName, + bool metaSet = true) + { + if (metaSet) + { + return generator.ForInstance(s => s.SetDefaultFiles(dataFileName)); + } + return generator.ForInstance(s => s.SetDefaultFilesWithoutMeta(dataFileName)); + } public static Generator WithFile( this Generator generator, @@ -98,6 +105,29 @@ public static InstanceSetters SetDefaultFiles( ) .Set(d => d.MetaFileId, (_, d) => d.MetaFile.Id); + public static InstanceSetters SetDefaultFilesWithoutMeta( + this InstanceSetters setters, + string dataFileName) + => setters + .Set( + d => d.File, + (_, d, context) => context.Fixture + .DefaultFile(FileType.Data) + .WithDataSetFileMeta(null) + .WithDataSetFileGeographicLevels([]) + .WithFilename($"{dataFileName}.csv") + .WithSubjectId(d.SubjectId) + ) + .Set(d => d.FileId, (_, d) => d.File.Id) + .Set( + d => d.MetaFile, + (_, d, context) => context.Fixture + .DefaultFile(FileType.Metadata) + .WithFilename($"{dataFileName}.meta.csv") + .WithSubjectId(d.SubjectId) + ) + .Set(d => d.MetaFileId, (_, d) => d.MetaFile.Id); + public static InstanceSetters SetFile( this InstanceSetters setters, File file) diff --git a/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/DataSetFileGeographicLevelGeneratorExtensions.cs b/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/DataSetFileGeographicLevelGeneratorExtensions.cs new file mode 100644 index 00000000000..b9c7d83a357 --- /dev/null +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/DataSetFileGeographicLevelGeneratorExtensions.cs @@ -0,0 +1,50 @@ +using System; +using GovUk.Education.ExploreEducationStatistics.Common.Model.Data; +using GovUk.Education.ExploreEducationStatistics.Common.Tests.Fixtures; + +namespace GovUk.Education.ExploreEducationStatistics.Content.Model.Tests.Fixtures; + +public static class DataSetFileGeographicLevelGeneratorExtensions +{ + public static Generator DefaultDataSetFileGeographicLevel(this DataFixture fixture) + => fixture.Generator().WithDefaults(); + + public static Generator WithDefaults(this Generator generator) + => generator.ForInstance(d => d.SetDefaults()); + + public static InstanceSetters SetDefaults( + this InstanceSetters setters) + => setters + .SetDataSetFileVersionId(Guid.NewGuid()) + .SetGeographicLevel(GeographicLevel.Country); + + public static Generator WithDataSetFileVersion( + this Generator generator, + File dataSetFileVersion) + => generator.ForInstance(s => s.SetDataSetFileVersion(dataSetFileVersion)); + + public static Generator WithDataSetFileVersionId( + this Generator generator, + Guid dataSetFileVersionId) + => generator.ForInstance(s => s.SetDataSetFileVersionId(dataSetFileVersionId)); + + public static Generator WithGeographicLevel( + this Generator generator, + GeographicLevel geographicLevel) + => generator.ForInstance(s => s.SetGeographicLevel(geographicLevel)); + + public static InstanceSetters SetDataSetFileVersion( + this InstanceSetters setters, + File dataSetFileVersion) + => setters.Set(f => f.DataSetFileVersion, dataSetFileVersion); + + public static InstanceSetters SetDataSetFileVersionId( + this InstanceSetters setters, + Guid dataSetFileVersionId) + => setters.Set(f => f.DataSetFileVersionId, dataSetFileVersionId); + + public static InstanceSetters SetGeographicLevel( + this InstanceSetters setters, + GeographicLevel geographicLevel) + => setters.Set(f => f.GeographicLevel, geographicLevel); +} diff --git a/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/FileGeneratorExtensions.cs b/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/FileGeneratorExtensions.cs index f45b0e99adc..b60fb4e6798 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/FileGeneratorExtensions.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/FileGeneratorExtensions.cs @@ -1,8 +1,9 @@ using System; +using System.Collections.Generic; +using System.Linq; using GovUk.Education.ExploreEducationStatistics.Common.Model; using GovUk.Education.ExploreEducationStatistics.Common.Model.Data; using GovUk.Education.ExploreEducationStatistics.Common.Tests.Fixtures; -using Semver; namespace GovUk.Education.ExploreEducationStatistics.Content.Model.Tests.Fixtures; @@ -84,6 +85,11 @@ public static Generator WithDataSetFileMeta( DataSetFileMeta? dataSetFileMeta) => generator.ForInstance(s => s.SetDataSetFileMeta(dataSetFileMeta)); + public static Generator WithDataSetFileGeographicLevels( + this Generator generator, + List geographicLevels) + => generator.ForInstance(s => s.SetDataSetFileGeographicLevels(geographicLevels)); + public static InstanceSetters SetDefaults(this InstanceSetters setters, FileType? fileType) => fileType switch { @@ -110,7 +116,8 @@ public static InstanceSetters SetDataFileDefaults(this InstanceSetters f.SubjectId) .SetContentType("text/csv") .SetDefault(f => f.DataSetFileId) - .Set(f => f.DataSetFileMeta, (_, _, context) => context.Fixture.DefaultDataSetFileMeta()); + .Set(f => f.DataSetFileMeta, (_, _, context) => context.Fixture.DefaultDataSetFileMeta()) + .SetDataSetFileGeographicLevels([GeographicLevel.Country, GeographicLevel.LocalAuthority, GeographicLevel.LocalAuthorityDistrict]); public static InstanceSetters SetMetaFileDefaults(this InstanceSetters setters) => setters @@ -200,4 +207,16 @@ public static InstanceSetters SetDataSetFileMeta( this InstanceSetters setters, DataSetFileMeta? dataSetFileMeta) => setters.Set(f => f.DataSetFileMeta, dataSetFileMeta); + + public static InstanceSetters SetDataSetFileGeographicLevels( + this InstanceSetters setters, + List geographicLevels) + => setters.Set( + file => file.DataSetFileGeographicLevels, + (_, file) => geographicLevels.Select( + gl => new DataSetFileGeographicLevel + { + DataSetFileVersionId = file.Id, + GeographicLevel = gl, + }).ToList()); } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Content.Model/File.cs b/src/GovUk.Education.ExploreEducationStatistics.Content.Model/File.cs index 8fc1696a46a..42c6f75b9dd 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.Model/File.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Model/File.cs @@ -23,7 +23,7 @@ public class File : ICreatedTimestamp public int? DataSetFileVersion { get; set; } - public List? DataSetFileGeographicLevels { get; set; } + public List DataSetFileGeographicLevels { get; set; } = []; public DataSetFileMeta? DataSetFileMeta { get; set; } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Content.Services/DataSetFileService.cs b/src/GovUk.Education.ExploreEducationStatistics.Content.Services/DataSetFileService.cs index 4a0775ccddd..b743fd1198e 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.Services/DataSetFileService.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Services/DataSetFileService.cs @@ -482,12 +482,12 @@ internal static IQueryable HavingReleaseVersionId( return releaseVersionId.HasValue ? query.Where(rf => rf.ReleaseVersionId == releaseVersionId.Value) : query; } - internal static IQueryable HavingGeographicLevel( // @MarkFix write tests + internal static IQueryable HavingGeographicLevel( this IQueryable query, GeographicLevel? geographicLevel) { return geographicLevel.HasValue - ? query.Where(rf => rf.File.DataSetFileGeographicLevels!.Any( // @MarkFix null allowing + ? query.Where(rf => rf.File.DataSetFileGeographicLevels.Any( gl => gl.GeographicLevel == geographicLevel && rf.FileId == gl.DataSetFileVersionId)) : query; diff --git a/src/GovUk.Education.ExploreEducationStatistics.Data.Processor.Tests/Functions/ProcessorStage3Tests.cs b/src/GovUk.Education.ExploreEducationStatistics.Data.Processor.Tests/Functions/ProcessorStage3Tests.cs index 36f3fdc9e87..965c0c781e1 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Data.Processor.Tests/Functions/ProcessorStage3Tests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Data.Processor.Tests/Functions/ProcessorStage3Tests.cs @@ -23,6 +23,7 @@ using GovUk.Education.ExploreEducationStatistics.Data.Processor.Services; using GovUk.Education.ExploreEducationStatistics.Data.Processor.Services.Interfaces; using GovUk.Education.ExploreEducationStatistics.Data.Processor.Tests.Services; +using GovUk.Education.ExploreEducationStatistics.Public.Data.Model.Migrations; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -106,7 +107,7 @@ public async Task ProcessStage3() var import = _fixture .DefaultDataImport() .WithSubjectId(_subject.Id) - .WithDefaultFiles("small-csv") + .WithDefaultFiles("small-csv", metaSet: false) .WithStatus(STAGE_3) .WithTotalRows(16) .WithExpectedImportedRows(16) @@ -325,7 +326,7 @@ public async Task ProcessStage3_FailsImportIfRowCountsDontMatch() var import = _fixture .DefaultDataImport() .WithSubjectId(_subject.Id) - .WithDefaultFiles("small-csv") + .WithDefaultFiles("small-csv", metaSet: false) .WithStatus(STAGE_3) .WithTotalRows(16) .WithExpectedImportedRows(16) @@ -454,7 +455,7 @@ public async Task ProcessStage3_PartiallyImportedAlready() var import = _fixture .DefaultDataImport() .WithSubjectId(_subject.Id) - .WithDefaultFiles("small-csv") + .WithDefaultFiles("small-csv", metaSet: false) .WithStatus(STAGE_3) .WithTotalRows(16) .WithExpectedImportedRows(16) @@ -604,7 +605,7 @@ public async Task ProcessStage3_PartiallyImportedAlready_BatchSizeChanged() var import = _fixture .DefaultDataImport() .WithSubjectId(_subject.Id) - .WithDefaultFiles("small-csv") + .WithDefaultFiles("small-csv", metaSet: false) .WithStatus(STAGE_3) .WithTotalRows(16) .WithExpectedImportedRows(16) @@ -749,7 +750,7 @@ public async Task ProcessStage3_IgnoredRows() var import = _fixture .DefaultDataImport() .WithSubjectId(_subject.Id) - .WithDefaultFiles("ignored-school-rows") + .WithDefaultFiles("ignored-school-rows", metaSet: false) .WithStatus(STAGE_3) .WithTotalRows(16) .WithExpectedImportedRows(8) @@ -894,7 +895,7 @@ public async Task ProcessStage3_IgnoredRows_PartiallyImported() var import = _fixture .DefaultDataImport() .WithSubjectId(_subject.Id) - .WithDefaultFiles("ignored-school-rows") + .WithDefaultFiles("ignored-school-rows", metaSet: false) .WithStatus(STAGE_3) .WithTotalRows(16) .WithExpectedImportedRows(8) @@ -1052,7 +1053,7 @@ public async Task ProcessStage3_Cancelling() var import = _fixture .DefaultDataImport() .WithSubjectId(_subject.Id) - .WithDefaultFiles("small-csv") + .WithDefaultFiles("small-csv", metaSet: false) .WithStatus(STAGE_3) .WithTotalRows(16) .WithExpectedImportedRows(16) @@ -1190,7 +1191,7 @@ public async Task ProcessStage3_CancelledAlready() var import = _fixture .DefaultDataImport() .WithSubjectId(_subject.Id) - .WithDefaultFiles("small-csv") + .WithDefaultFiles("small-csv", metaSet: false) .WithStatus(CANCELLED) .WithTotalRows(16) .WithExpectedImportedRows(16) @@ -1324,7 +1325,7 @@ public async Task ProcessStage3_AdditionalFiltersAndIndicators() var import = _fixture .DefaultDataImport() .WithSubjectId(_subject.Id) - .WithDefaultFiles("additional-filters-and-indicators") + .WithDefaultFiles("additional-filters-and-indicators", metaSet: false) .WithStatus(STAGE_3) .WithTotalRows(16) .WithExpectedImportedRows(16) @@ -1493,7 +1494,7 @@ public async Task ProcessStage3_SpecialFilterItemAndIndicatorValues() var import = _fixture .DefaultDataImport() .WithSubjectId(subject.Id) - .WithDefaultFiles("small-csv-with-special-data") + .WithDefaultFiles("small-csv-with-special-data", metaSet: false) .WithStatus(STAGE_3) .WithTotalRows(5) .WithExpectedImportedRows(5) diff --git a/src/GovUk.Education.ExploreEducationStatistics.Data.Processor.Tests/Services/DataImportServiceTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Data.Processor.Tests/Services/DataImportServiceTests.cs index 5aa9ee86309..8463051e0f5 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Data.Processor.Tests/Services/DataImportServiceTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Data.Processor.Tests/Services/DataImportServiceTests.cs @@ -220,6 +220,7 @@ public async Task WriteDataSetMetaFile_Success() var file = _fixture.DefaultFile(FileType.Data) .WithDataSetFileMeta(null) + .WithDataSetFileGeographicLevels([]) .WithSubjectId(subject.Id) .Generate(); diff --git a/src/GovUk.Education.ExploreEducationStatistics.Data.Processor/Services/DataImportService.cs b/src/GovUk.Education.ExploreEducationStatistics.Data.Processor/Services/DataImportService.cs index acb15120605..162c826776d 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Data.Processor/Services/DataImportService.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Data.Processor/Services/DataImportService.cs @@ -222,6 +222,7 @@ public async Task WriteDataSetFileMeta(Guid fileId, Guid subjectId) file.DataSetFileMeta = dataSetFileMeta; var dataSetFileGeographicLevels = geographicLevels + .Distinct() .Select(gl => new DataSetFileGeographicLevel { DataSetFileVersionId = fileId, diff --git a/src/explore-education-statistics-frontend/src/modules/data-catalogue/__data__/testDataSets.ts b/src/explore-education-statistics-frontend/src/modules/data-catalogue/__data__/testDataSets.ts index c73c983240c..c6a2e4a13ca 100644 --- a/src/explore-education-statistics-frontend/src/modules/data-catalogue/__data__/testDataSets.ts +++ b/src/explore-education-statistics-frontend/src/modules/data-catalogue/__data__/testDataSets.ts @@ -77,7 +77,7 @@ export const testDataSetFileSummaries: DataSetFileSummary[] = [ to: '2020', }, filters: ['Filter 1', 'Filter 2'], - geographicLevels: ['National', 'Regional'], + geographicLevels: ['National', 'Regional', 'Local authority'], indicators: ['Indicator 1', 'Indicator 2'], }, latestData: true, diff --git a/src/explore-education-statistics-frontend/src/modules/data-catalogue/__tests__/DataCataloguePage.test.tsx b/src/explore-education-statistics-frontend/src/modules/data-catalogue/__tests__/DataCataloguePage.test.tsx index 4026e9df8c6..e6b684958f7 100644 --- a/src/explore-education-statistics-frontend/src/modules/data-catalogue/__tests__/DataCataloguePage.test.tsx +++ b/src/explore-education-statistics-frontend/src/modules/data-catalogue/__tests__/DataCataloguePage.test.tsx @@ -1195,6 +1195,32 @@ describe('DataCataloguePage', () => { ).toBeInTheDocument(); }); + test('filters by geographic level', async () => { + mockRouter.setCurrentUrl('/data-catalogue?geographicLevel=LA'); + + dataSetService.listDataSetFiles.mockResolvedValueOnce({ + results: [testDataSetFileSummaries[1], testDataSetFileSummaries[2]], + paging: { ...testPaging, totalPages: 1, totalResults: 1 }, + }); + publicationService.getPublicationTree.mockResolvedValue(testThemes); + publicationService.listReleases.mockResolvedValue(testReleases); + + render(); + + await waitFor(() => { + expect(screen.getByText('1 data set')).toBeInTheDocument(); + }); + + expect(mockRouter).toMatchObject({ + pathname: '/data-catalogue', + query: { geographicLevel: 'LA' }, + }); + + expect(screen.getByLabelText('Filter by Geographic level')).toHaveValue( + 'LA', + ); + }); + test('filters by search term', async () => { mockRouter.setCurrentUrl('/data-catalogue?searchTerm=find+me'); diff --git a/src/explore-education-statistics-frontend/src/modules/data-catalogue/components/__tests__/DataSetSummary.test.tsx b/src/explore-education-statistics-frontend/src/modules/data-catalogue/components/__tests__/DataSetSummary.test.tsx index ddf431535b5..fd464e53a87 100644 --- a/src/explore-education-statistics-frontend/src/modules/data-catalogue/components/__tests__/DataSetSummary.test.tsx +++ b/src/explore-education-statistics-frontend/src/modules/data-catalogue/components/__tests__/DataSetSummary.test.tsx @@ -50,7 +50,7 @@ describe('DataSetFileSummary', () => { expect( within(screen.getByTestId('Geographic levels')).getByText( - 'National, Regional', + 'Local authority, National, Regional', ), ).toBeInTheDocument(); expect( @@ -83,7 +83,7 @@ describe('DataSetFileSummary', () => { expect( within(screen.getByTestId('Geographic levels')).getByText( - 'National, Regional', + 'Local authority, National, Regional', ), ).toBeInTheDocument(); expect(