From 1d65b4b418c6ff75ea282e1fb3ba8c0799af53d4 Mon Sep 17 00:00:00 2001 From: Mark Youngman Date: Tue, 17 Dec 2024 12:23:51 +0000 Subject: [PATCH] EES-5738 Changes in response to PR comments --- ...FileGeographicLevelsMigrationController.cs | 14 ++--- ...aSetFileGeographicLevelsTable.Designer.cs} | 15 +++--- ...CreateDataSetFileGeographicLevelsTable.cs} | 22 +++++--- .../ContentDbContextModelSnapshot.cs | 13 ++--- .../DataSetFilesControllerTests.cs | 22 +++----- .../Fixtures/DataImportGeneratorExtensions.cs | 2 +- ...tFileGeographicLevelGeneratorExtensions.cs | 50 ------------------ ...rsionGeographicLevelGeneratorExtensions.cs | 51 +++++++++++++++++++ .../Fixtures/FileGeneratorExtensions.cs | 13 ++--- ...s => DataSetFileVersionGeographicLevel.cs} | 2 +- .../Database/ContentDbContext.cs | 11 ++-- .../File.cs | 2 +- .../DataSetFileService.cs | 5 +- .../Services/DataImportServiceTests.cs | 2 +- .../Services/DataImportService.cs | 8 +-- .../src/utils/locationLevelsMap.ts | 19 +++++++ .../data-catalogue/DataCataloguePage.tsx | 22 +++++++- .../data-catalogue/components/Filters.tsx | 4 +- .../admin_and_public/bau/data_catalogue.robot | 16 ++++++ 19 files changed, 177 insertions(+), 116 deletions(-) rename src/GovUk.Education.ExploreEducationStatistics.Admin/Migrations/ContentMigrations/{20241213123442_EES5738_CreateDataSetFileGeographicLevelsTable.Designer.cs => 20241219093820_EES5738_CreateDataSetFileGeographicLevelsTable.Designer.cs} (99%) rename src/GovUk.Education.ExploreEducationStatistics.Admin/Migrations/ContentMigrations/{20241213123442_EES5738_CreateDataSetFileGeographicLevelsTable.cs => 20241219093820_EES5738_CreateDataSetFileGeographicLevelsTable.cs} (62%) delete mode 100644 src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/DataSetFileGeographicLevelGeneratorExtensions.cs create mode 100644 src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/DataSetFileVersionGeographicLevelGeneratorExtensions.cs rename src/GovUk.Education.ExploreEducationStatistics.Content.Model/{DataSetFileGeographicLevels.cs => DataSetFileVersionGeographicLevel.cs} (89%) diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin/Controllers/Api/DataSetFileGeographicLevelsMigrationController.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin/Controllers/Api/DataSetFileGeographicLevelsMigrationController.cs index f1adf49fd34..b4ceec2eb98 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin/Controllers/Api/DataSetFileGeographicLevelsMigrationController.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin/Controllers/Api/DataSetFileGeographicLevelsMigrationController.cs @@ -4,6 +4,7 @@ using System.Threading.Tasks; using GovUk.Education.ExploreEducationStatistics.Admin.Models; using GovUk.Education.ExploreEducationStatistics.Common.Extensions; +using GovUk.Education.ExploreEducationStatistics.Common.Model; using GovUk.Education.ExploreEducationStatistics.Content.Model; using GovUk.Education.ExploreEducationStatistics.Content.Model.Database; using Microsoft.AspNetCore.Authorization; @@ -24,14 +25,14 @@ public class MigrationResult } [HttpPut("bau/migrate-datasetfile-geographiclevels")] - public async Task DataSetFileGeographicLevelsMigration( + public async Task DataSetFileVersionGeographicLevelsMigration( [FromQuery] bool isDryRun = true, [FromQuery] int? num = null, CancellationToken cancellationToken = default) { var queryable = contentDbContext.Files - .Where(f => f.DataSetFileMeta != null - && f.DataSetFileGeographicLevels.Count == 0); + .Where(f => f.Type == FileType.Data + && f.DataSetFileVersionGeographicLevels.Count == 0); if (num != null) { @@ -53,16 +54,17 @@ public async Task DataSetFileGeographicLevelsMigration( continue; } - var dataSetFileGeographicLevels = meta!.GeographicLevels + var dataSetFileVersionGeographicLevels = meta!.GeographicLevels .Distinct() - .Select(gl => new DataSetFileGeographicLevel + .Select(gl => new DataSetFileVersionGeographicLevel { DataSetFileVersionId = file.Id, GeographicLevel = gl, }) .ToList(); - contentDbContext.DataSetFileGeographicLevels.AddRange(dataSetFileGeographicLevels); + contentDbContext.DataSetFileVersionGeographicLevels.AddRange( + dataSetFileVersionGeographicLevels); numProcessed++; } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin/Migrations/ContentMigrations/20241213123442_EES5738_CreateDataSetFileGeographicLevelsTable.Designer.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin/Migrations/ContentMigrations/20241219093820_EES5738_CreateDataSetFileGeographicLevelsTable.Designer.cs similarity index 99% rename from src/GovUk.Education.ExploreEducationStatistics.Admin/Migrations/ContentMigrations/20241213123442_EES5738_CreateDataSetFileGeographicLevelsTable.Designer.cs rename to src/GovUk.Education.ExploreEducationStatistics.Admin/Migrations/ContentMigrations/20241219093820_EES5738_CreateDataSetFileGeographicLevelsTable.Designer.cs index 9c4bfa60215..739bb0e01e7 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin/Migrations/ContentMigrations/20241213123442_EES5738_CreateDataSetFileGeographicLevelsTable.Designer.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin/Migrations/ContentMigrations/20241219093820_EES5738_CreateDataSetFileGeographicLevelsTable.Designer.cs @@ -12,7 +12,7 @@ namespace GovUk.Education.ExploreEducationStatistics.Admin.Migrations.ContentMigrations { [DbContext(typeof(ContentDbContext))] - [Migration("20241213123442_EES5738_CreateDataSetFileGeographicLevelsTable")] + [Migration("20241219093820_EES5738_CreateDataSetFileGeographicLevelsTable")] partial class EES5738_CreateDataSetFileGeographicLevelsTable { /// @@ -331,17 +331,18 @@ protected override void BuildTargetModel(ModelBuilder modelBuilder) b.ToTable("DataImportErrors"); }); - modelBuilder.Entity("GovUk.Education.ExploreEducationStatistics.Content.Model.DataSetFileGeographicLevel", b => + modelBuilder.Entity("GovUk.Education.ExploreEducationStatistics.Content.Model.DataSetFileVersionGeographicLevel", b => { b.Property("DataSetFileVersionId") .HasColumnType("uniqueidentifier"); b.Property("GeographicLevel") - .HasColumnType("nvarchar(450)"); + .HasMaxLength(6) + .HasColumnType("nvarchar(6)"); b.HasKey("DataSetFileVersionId", "GeographicLevel"); - b.ToTable("DataSetFileGeographicLevels"); + b.ToTable("DataSetFileVersionGeographicLevels"); }); modelBuilder.Entity("GovUk.Education.ExploreEducationStatistics.Content.Model.EmbedBlock", b => @@ -1610,10 +1611,10 @@ protected override void BuildTargetModel(ModelBuilder modelBuilder) b.Navigation("DataImport"); }); - modelBuilder.Entity("GovUk.Education.ExploreEducationStatistics.Content.Model.DataSetFileGeographicLevel", b => + modelBuilder.Entity("GovUk.Education.ExploreEducationStatistics.Content.Model.DataSetFileVersionGeographicLevel", b => { b.HasOne("GovUk.Education.ExploreEducationStatistics.Content.Model.File", "DataSetFileVersion") - .WithMany("DataSetFileGeographicLevels") + .WithMany("DataSetFileVersionGeographicLevels") .HasForeignKey("DataSetFileVersionId") .OnDelete(DeleteBehavior.Cascade) .IsRequired(); @@ -2209,7 +2210,7 @@ protected override void BuildTargetModel(ModelBuilder modelBuilder) modelBuilder.Entity("GovUk.Education.ExploreEducationStatistics.Content.Model.File", b => { - b.Navigation("DataSetFileGeographicLevels"); + b.Navigation("DataSetFileVersionGeographicLevels"); }); modelBuilder.Entity("GovUk.Education.ExploreEducationStatistics.Content.Model.Methodology", b => diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin/Migrations/ContentMigrations/20241213123442_EES5738_CreateDataSetFileGeographicLevelsTable.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin/Migrations/ContentMigrations/20241219093820_EES5738_CreateDataSetFileGeographicLevelsTable.cs similarity index 62% rename from src/GovUk.Education.ExploreEducationStatistics.Admin/Migrations/ContentMigrations/20241213123442_EES5738_CreateDataSetFileGeographicLevelsTable.cs rename to src/GovUk.Education.ExploreEducationStatistics.Admin/Migrations/ContentMigrations/20241219093820_EES5738_CreateDataSetFileGeographicLevelsTable.cs index 1434036a2a3..7a652fbef83 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin/Migrations/ContentMigrations/20241213123442_EES5738_CreateDataSetFileGeographicLevelsTable.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin/Migrations/ContentMigrations/20241219093820_EES5738_CreateDataSetFileGeographicLevelsTable.cs @@ -1,6 +1,8 @@ using System; using Microsoft.EntityFrameworkCore.Migrations; +#nullable disable + namespace GovUk.Education.ExploreEducationStatistics.Admin.Migrations.ContentMigrations { /// @@ -10,25 +12,27 @@ public partial class EES5738_CreateDataSetFileGeographicLevelsTable : Migration protected override void Up(MigrationBuilder migrationBuilder) { migrationBuilder.CreateTable( - name: "DataSetFileGeographicLevels", + name: "DataSetFileVersionGeographicLevels", columns: table => new { DataSetFileVersionId = table.Column(type: "uniqueidentifier", nullable: false), - GeographicLevel = table.Column(type: "nvarchar(450)", nullable: false) + GeographicLevel = table.Column(type: "nvarchar(6)", maxLength: 6, nullable: false) }, constraints: table => { - table.PrimaryKey("PK_DataSetFileGeographicLevels", x => new { x.DataSetFileVersionId, x.GeographicLevel }); + table.PrimaryKey("PK_DataSetFileVersionGeographicLevels", x => new { x.DataSetFileVersionId, x.GeographicLevel }); table.ForeignKey( - name: "FK_DataSetFileGeographicLevels_Files_DataSetFileVersionId", + name: "FK_DataSetFileVersionGeographicLevels_Files_DataSetFileVersionId", column: x => x.DataSetFileVersionId, principalTable: "Files", principalColumn: "Id", onDelete: ReferentialAction.Cascade); }); + migrationBuilder.Sql("GRANT SELECT ON dbo.DataSetFileVersionGeographicLevels TO [content];"); + migrationBuilder.Sql("GRANT INSERT ON dbo.DataSetFileVersionGeographicLevels TO [importer];"); - migrationBuilder.Sql("GRANT SELECT ON dbo.DataSetFileGeographicLevels TO [content];"); - migrationBuilder.Sql("GRANT INSERT ON dbo.DataSetFileGeographicLevels TO [importer];"); + migrationBuilder.Sql("GRANT SELECT ON dbo.DataSetFileVersionGeographicLevels TO [data];"); + migrationBuilder.Sql("GRANT SELECT ON dbo.DataSetFileVersionGeographicLevels TO [publisher];"); } /// @@ -36,8 +40,12 @@ protected override void Down(MigrationBuilder migrationBuilder) { migrationBuilder.Sql("REVOKE SELECT ON dbo.DataSetFileGeographicLevels TO [content];"); migrationBuilder.Sql("REVOKE INSERT ON dbo.DataSetFileGeographicLevels TO [importer];"); + + migrationBuilder.Sql("REVOKE SELECT ON dbo.DataSetFileGeographicLevels TO [data];"); + migrationBuilder.Sql("REVOKE SELECT ON dbo.DataSetFileGeographicLevels TO [publisher];"); + migrationBuilder.DropTable( - name: "DataSetFileGeographicLevels"); + name: "DataSetFileVersionGeographicLevels"); } } } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin/Migrations/ContentMigrations/ContentDbContextModelSnapshot.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin/Migrations/ContentMigrations/ContentDbContextModelSnapshot.cs index bc677140783..292daec9d7f 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin/Migrations/ContentMigrations/ContentDbContextModelSnapshot.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin/Migrations/ContentMigrations/ContentDbContextModelSnapshot.cs @@ -328,17 +328,18 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.ToTable("DataImportErrors"); }); - modelBuilder.Entity("GovUk.Education.ExploreEducationStatistics.Content.Model.DataSetFileGeographicLevel", b => + modelBuilder.Entity("GovUk.Education.ExploreEducationStatistics.Content.Model.DataSetFileVersionGeographicLevel", b => { b.Property("DataSetFileVersionId") .HasColumnType("uniqueidentifier"); b.Property("GeographicLevel") - .HasColumnType("nvarchar(450)"); + .HasMaxLength(6) + .HasColumnType("nvarchar(6)"); b.HasKey("DataSetFileVersionId", "GeographicLevel"); - b.ToTable("DataSetFileGeographicLevels"); + b.ToTable("DataSetFileVersionGeographicLevels"); }); modelBuilder.Entity("GovUk.Education.ExploreEducationStatistics.Content.Model.EmbedBlock", b => @@ -1607,10 +1608,10 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.Navigation("DataImport"); }); - modelBuilder.Entity("GovUk.Education.ExploreEducationStatistics.Content.Model.DataSetFileGeographicLevel", b => + modelBuilder.Entity("GovUk.Education.ExploreEducationStatistics.Content.Model.DataSetFileVersionGeographicLevel", b => { b.HasOne("GovUk.Education.ExploreEducationStatistics.Content.Model.File", "DataSetFileVersion") - .WithMany("DataSetFileGeographicLevels") + .WithMany("DataSetFileVersionGeographicLevels") .HasForeignKey("DataSetFileVersionId") .OnDelete(DeleteBehavior.Cascade) .IsRequired(); @@ -2206,7 +2207,7 @@ protected override void BuildModel(ModelBuilder modelBuilder) modelBuilder.Entity("GovUk.Education.ExploreEducationStatistics.Content.Model.File", b => { - b.Navigation("DataSetFileGeographicLevels"); + b.Navigation("DataSetFileVersionGeographicLevels"); }); modelBuilder.Entity("GovUk.Education.ExploreEducationStatistics.Content.Model.Methodology", b => 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 b69ddb03ce6..b32dde93656 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.Api.Tests/Controllers/DataSetFilesControllerTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Api.Tests/Controllers/DataSetFilesControllerTests.cs @@ -101,27 +101,21 @@ public async Task FilterByGeographicLevel_Success() var (publication1, publication2) = _fixture .DefaultPublication() // Publications each have a published release version - .WithReleases(_fixture.DefaultRelease(publishedVersions: 1) - .Generate(1)) + .WithReleases([_fixture.DefaultRelease(publishedVersions: 1)]) .WithTheme(_fixture.DefaultTheme()) .GenerateTuple2(); - var publication1Release1Version1Files = _fixture.DefaultReleaseFile() + ReleaseFile publication1Release1Version1File = _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(); + .WithFile(_fixture.DefaultFile(FileType.Data) + .WithDataSetFileVersionGeographicLevels( + [GeographicLevel.Country, GeographicLevel.LocalAuthority, GeographicLevel.Institution])); var publication2Release1Version1Files = GenerateDataSetFilesForReleaseVersion(publication2.ReleaseVersions[0]); await TestApp.AddTestData(context => { - context.ReleaseFiles.AddRange(publication1Release1Version1Files); + context.ReleaseFiles.Add(publication1Release1Version1File); context.ReleaseFiles.AddRange(publication2Release1Version1Files); }); @@ -137,7 +131,7 @@ await TestApp.AddTestData(context => pagedResult.AssertHasExpectedPagingAndResultCount( expectedTotalResults: 1); - AssertResultsForExpectedReleaseFiles(publication1Release1Version1Files, pagedResult.Results); + AssertResultsForExpectedReleaseFiles([publication1Release1Version1File], pagedResult.Results); } [Fact] @@ -1783,7 +1777,7 @@ private List GenerateDataSetFilesForReleaseVersion( .WithReleaseVersion(releaseVersion) .WithFiles(_fixture.DefaultFile(FileType.Data) .WithDataSetFileMeta(_fixture.DefaultDataSetFileMeta()) - .WithDataSetFileGeographicLevels([GeographicLevel.Country]) + .WithDataSetFileVersionGeographicLevels([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 0eab870e422..5d13f4f4164 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/DataImportGeneratorExtensions.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/DataImportGeneratorExtensions.cs @@ -114,7 +114,7 @@ public static InstanceSetters SetDefaultFilesWithoutMeta( (_, d, context) => context.Fixture .DefaultFile(FileType.Data) .WithDataSetFileMeta(null) - .WithDataSetFileGeographicLevels([]) + .WithDataSetFileVersionGeographicLevels([]) .WithFilename($"{dataFileName}.csv") .WithSubjectId(d.SubjectId) ) diff --git a/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/DataSetFileGeographicLevelGeneratorExtensions.cs b/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/DataSetFileGeographicLevelGeneratorExtensions.cs deleted file mode 100644 index b9c7d83a357..00000000000 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/DataSetFileGeographicLevelGeneratorExtensions.cs +++ /dev/null @@ -1,50 +0,0 @@ -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/DataSetFileVersionGeographicLevelGeneratorExtensions.cs b/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/DataSetFileVersionGeographicLevelGeneratorExtensions.cs new file mode 100644 index 00000000000..eb49d2897ec --- /dev/null +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/DataSetFileVersionGeographicLevelGeneratorExtensions.cs @@ -0,0 +1,51 @@ +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 DataSetFileVersionGeographicLevelGeneratorExtensions +{ + public static Generator DefaultDataSetFileVersionGeographicLevel(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 + .SetDefault(p => p.DataSetFileVersionId); + + 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) + .Set(f => f.DataSetFileVersionId, dataSetFileVersion.Id); + + 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 b60fb4e6798..320c2950442 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/FileGeneratorExtensions.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/FileGeneratorExtensions.cs @@ -85,10 +85,10 @@ public static Generator WithDataSetFileMeta( DataSetFileMeta? dataSetFileMeta) => generator.ForInstance(s => s.SetDataSetFileMeta(dataSetFileMeta)); - public static Generator WithDataSetFileGeographicLevels( + public static Generator WithDataSetFileVersionGeographicLevels( this Generator generator, List geographicLevels) - => generator.ForInstance(s => s.SetDataSetFileGeographicLevels(geographicLevels)); + => generator.ForInstance(s => s.SetDataSetFileVersionGeographicLevels(geographicLevels)); public static InstanceSetters SetDefaults(this InstanceSetters setters, FileType? fileType) => fileType switch @@ -117,7 +117,7 @@ public static InstanceSetters SetDataFileDefaults(this InstanceSetters f.DataSetFileId) .Set(f => f.DataSetFileMeta, (_, _, context) => context.Fixture.DefaultDataSetFileMeta()) - .SetDataSetFileGeographicLevels([GeographicLevel.Country, GeographicLevel.LocalAuthority, GeographicLevel.LocalAuthorityDistrict]); + .SetDataSetFileVersionGeographicLevels([GeographicLevel.Country, GeographicLevel.LocalAuthority, GeographicLevel.LocalAuthorityDistrict]); public static InstanceSetters SetMetaFileDefaults(this InstanceSetters setters) => setters @@ -208,15 +208,16 @@ public static InstanceSetters SetDataSetFileMeta( DataSetFileMeta? dataSetFileMeta) => setters.Set(f => f.DataSetFileMeta, dataSetFileMeta); - public static InstanceSetters SetDataSetFileGeographicLevels( + public static InstanceSetters SetDataSetFileVersionGeographicLevels( this InstanceSetters setters, List geographicLevels) => setters.Set( - file => file.DataSetFileGeographicLevels, + file => file.DataSetFileVersionGeographicLevels, (_, file) => geographicLevels.Select( - gl => new DataSetFileGeographicLevel + gl => new DataSetFileVersionGeographicLevel { DataSetFileVersionId = file.Id, + DataSetFileVersion = file, GeographicLevel = gl, }).ToList()); } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Content.Model/DataSetFileGeographicLevels.cs b/src/GovUk.Education.ExploreEducationStatistics.Content.Model/DataSetFileVersionGeographicLevel.cs similarity index 89% rename from src/GovUk.Education.ExploreEducationStatistics.Content.Model/DataSetFileGeographicLevels.cs rename to src/GovUk.Education.ExploreEducationStatistics.Content.Model/DataSetFileVersionGeographicLevel.cs index 2b23db07a4a..d677283dbe6 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.Model/DataSetFileGeographicLevels.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Model/DataSetFileVersionGeographicLevel.cs @@ -4,7 +4,7 @@ namespace GovUk.Education.ExploreEducationStatistics.Content.Model; -public class DataSetFileGeographicLevel +public class DataSetFileVersionGeographicLevel { public Guid DataSetFileVersionId { get; set; } // Currently Files.Id, but will become DataSetFileVersion.Id in EES-5105 diff --git a/src/GovUk.Education.ExploreEducationStatistics.Content.Model/Database/ContentDbContext.cs b/src/GovUk.Education.ExploreEducationStatistics.Content.Model/Database/ContentDbContext.cs index 2cec48a76d7..8521a3ede07 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.Model/Database/ContentDbContext.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Model/Database/ContentDbContext.cs @@ -55,7 +55,7 @@ private void Configure(bool updateTimestamps = true) public virtual DbSet ReleaseStatus { get; set; } public virtual DbSet ReleaseFiles { get; set; } public virtual DbSet Files { get; set; } - public virtual DbSet DataSetFileGeographicLevels { get; set; } + public virtual DbSet DataSetFileVersionGeographicLevels { get; set; } public virtual DbSet ContentSections { get; set; } public virtual DbSet ContentBlocks { get; set; } public virtual DbSet KeyStatistics { get; set; } @@ -111,7 +111,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) ConfigureReleaseStatus(modelBuilder); ConfigureReleaseFile(modelBuilder); ConfigureFile(modelBuilder); - ConfigureDataSetFileGeographicLevel(modelBuilder); + ConfigureDataSetFileVersionGeographicLevel(modelBuilder); ConfigureContentBlock(modelBuilder); ConfigureContentSection(modelBuilder); ConfigureReleaseVersion(modelBuilder); @@ -454,16 +454,16 @@ private static void ConfigureFile(ModelBuilder modelBuilder) v => v.HasValue ? DateTime.SpecifyKind(v.Value, DateTimeKind.Utc) : null); - entity.Property(p => p.DataSetFileMeta) // EES-5666 + entity.Property(p => p.DataSetFileMeta) .HasConversion( v => JsonConvert.SerializeObject(v), v => JsonConvert.DeserializeObject(v)); }); } - private static void ConfigureDataSetFileGeographicLevel(ModelBuilder modelBuilder) + private static void ConfigureDataSetFileVersionGeographicLevel(ModelBuilder modelBuilder) { - modelBuilder.Entity(entity => + modelBuilder.Entity(entity => { entity.HasKey(gl => new { @@ -472,6 +472,7 @@ private static void ConfigureDataSetFileGeographicLevel(ModelBuilder modelBuilde }); entity.Property(gl => gl.GeographicLevel) + .HasMaxLength(6) .HasConversion(new EnumToEnumValueConverter()); }); } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Content.Model/File.cs b/src/GovUk.Education.ExploreEducationStatistics.Content.Model/File.cs index 42c6f75b9dd..7ada31a8934 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 DataSetFileVersionGeographicLevels { 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 b743fd1198e..e6d0e9d85d9 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.Services/DataSetFileService.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Services/DataSetFileService.cs @@ -487,9 +487,8 @@ internal static IQueryable HavingGeographicLevel( GeographicLevel? geographicLevel) { return geographicLevel.HasValue - ? query.Where(rf => rf.File.DataSetFileGeographicLevels.Any( - gl => gl.GeographicLevel == geographicLevel - && rf.FileId == gl.DataSetFileVersionId)) + ? query.Where(rf => rf.File.DataSetFileVersionGeographicLevels.Any( + gl => gl.GeographicLevel == geographicLevel)) : query; } 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 8463051e0f5..de19c60ad62 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Data.Processor.Tests/Services/DataImportServiceTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Data.Processor.Tests/Services/DataImportServiceTests.cs @@ -220,7 +220,7 @@ public async Task WriteDataSetMetaFile_Success() var file = _fixture.DefaultFile(FileType.Data) .WithDataSetFileMeta(null) - .WithDataSetFileGeographicLevels([]) + .WithDataSetFileVersionGeographicLevels([]) .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 162c826776d..5b29fd83184 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Data.Processor/Services/DataImportService.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Data.Processor/Services/DataImportService.cs @@ -221,14 +221,14 @@ public async Task WriteDataSetFileMeta(Guid fileId, Guid subjectId) && f.SubjectId == subjectId); file.DataSetFileMeta = dataSetFileMeta; - var dataSetFileGeographicLevels = geographicLevels - .Distinct() - .Select(gl => new DataSetFileGeographicLevel + var dataSetFileVersionGeographicLevels = geographicLevels + .Select(gl => new DataSetFileVersionGeographicLevel { DataSetFileVersionId = fileId, GeographicLevel = gl, }).ToList(); - await contentDbContext.DataSetFileGeographicLevels.AddRangeAsync(dataSetFileGeographicLevels); + contentDbContext.DataSetFileVersionGeographicLevels.AddRange( + dataSetFileVersionGeographicLevels); await contentDbContext.SaveChangesAsync(); } diff --git a/src/explore-education-statistics-common/src/utils/locationLevelsMap.ts b/src/explore-education-statistics-common/src/utils/locationLevelsMap.ts index 6f54b04f835..947dd7b3049 100644 --- a/src/explore-education-statistics-common/src/utils/locationLevelsMap.ts +++ b/src/explore-education-statistics-common/src/utils/locationLevelsMap.ts @@ -22,6 +22,7 @@ export type GeographicLevelCode = | 'WARD'; export interface LocationLevelDetails { + filterLabel: string; // used in the dropdown that filters data sets by geographic level label: string; plural: string; prefix: string; @@ -30,108 +31,126 @@ export interface LocationLevelDetails { const locationLevelsMap = { country: { + filterLabel: 'National', label: 'Country', plural: 'Countries', prefix: 'a', code: 'NAT', }, englishDevolvedArea: { + filterLabel: 'English Devolved Area', label: 'English Devolved Area', plural: 'English Devolved Areas', prefix: 'an', code: 'EDA', }, institution: { + filterLabel: 'Institution', label: 'Institution', plural: 'Institutions', prefix: 'an', code: 'INST', }, localAuthority: { + filterLabel: 'Local Authority', label: 'Local Authority', plural: 'Local Authorities', prefix: 'a', code: 'LA', }, localAuthorityDistrict: { + filterLabel: 'Local Authority District', label: 'Local Authority District', plural: 'Local Authority Districts', prefix: 'a', code: 'LAD', }, localEnterprisePartnership: { + filterLabel: 'Local Enterprise Partnership', label: 'Local Enterprise Partnership', plural: 'Local Enterprise Partnerships', prefix: 'a', code: 'LEP', }, localSkillsImprovementPlanArea: { + filterLabel: 'Local Skills Improvement Plan Area', label: 'Local Skills Improvement Plan Area', plural: 'Local Skills Improvement Plan Areas', prefix: 'a', code: 'LSIP', }, mayoralCombinedAuthority: { + filterLabel: 'Mayoral Combined Authority', label: 'Mayoral Combined Authority', plural: 'Mayoral Combined Authorities', prefix: 'a', code: 'MCA', }, multiAcademyTrust: { + filterLabel: 'Multi Academy Trust', label: 'Multi Academy Trust', plural: 'Multi Academy Trusts', prefix: 'a', code: 'MAT', }, opportunityArea: { + filterLabel: 'Opportunity Area', label: 'Opportunity Area', plural: 'Opportunity Areas', prefix: 'an', code: 'OA', }, parliamentaryConstituency: { + filterLabel: 'Parliamentary Constituency', label: 'Parliamentary Constituency', plural: 'Parliamentary Constituencies', prefix: 'a', code: 'PCON', }, planningArea: { + filterLabel: 'Planning Area', label: 'Planning Area', plural: 'Planning Areas', prefix: 'a', code: 'PA', }, provider: { + filterLabel: 'Provider', label: 'Provider', plural: 'Providers', prefix: 'a', code: 'PROV', }, region: { + filterLabel: 'Region', label: 'Region', plural: 'Regions', prefix: 'a', code: 'REG', }, rscRegion: { + filterLabel: 'RSC Region', label: 'RSC Region', plural: 'RSC Regions', prefix: 'an', code: 'RSC', }, school: { + filterLabel: 'School', label: 'School', plural: 'Schools', prefix: 'a', code: 'SCH', }, sponsor: { + filterLabel: 'Sponsor', label: 'Sponsor', plural: 'Sponsors', prefix: 'a', code: 'SPON', }, ward: { + filterLabel: 'Ward', label: 'Ward', plural: 'Wards', prefix: 'a', diff --git a/src/explore-education-statistics-frontend/src/modules/data-catalogue/DataCataloguePage.tsx b/src/explore-education-statistics-frontend/src/modules/data-catalogue/DataCataloguePage.tsx index c14872aa2e0..039b1dd1c12 100644 --- a/src/explore-education-statistics-frontend/src/modules/data-catalogue/DataCataloguePage.tsx +++ b/src/explore-education-statistics-frontend/src/modules/data-catalogue/DataCataloguePage.tsx @@ -52,7 +52,11 @@ import omit from 'lodash/omit'; import Head from 'next/head'; import { useRouter } from 'next/router'; import { ParsedUrlQuery } from 'querystring'; -import { GeographicLevelCode } from '@common/utils/locationLevelsMap'; +import locationLevelsMap, { + GeographicLevelCode, + geographicLevelCodesMap, + LocationLevelKey, +} from '@common/utils/locationLevelsMap'; const defaultPageTitle = 'Data catalogue'; @@ -124,11 +128,16 @@ const DataCataloguePage: NextPage = ({ showTypeFilter }) => { const selectedRelease = releases.find(release => release.id === releaseId); + const selectedGeographicLevel = geographicLevel + ? geographicLevelCodesMap[geographicLevel] + : undefined; + const { paging, results: dataSets = [] } = dataSetsData ?? {}; const { page, totalPages, totalResults = 0 } = paging ?? {}; const [showAllDetails, toggleAllDetails] = useToggle(false); - const isFiltered = !!publicationId || !!searchTerm || !!themeId; + const isFiltered = + !!publicationId || !!searchTerm || !!themeId || !!geographicLevel; const filteredByString = compact([ searchTerm, @@ -420,6 +429,15 @@ const DataCataloguePage: NextPage = ({ showTypeFilter }) => { } /> )} + {selectedGeographicLevel && ( + + handleResetFilter({ filterType: 'geographicLevel' }) + } + /> + )} )} diff --git a/src/explore-education-statistics-frontend/src/modules/data-catalogue/components/Filters.tsx b/src/explore-education-statistics-frontend/src/modules/data-catalogue/components/Filters.tsx index c564cbc8fd5..a5e34d16eb0 100644 --- a/src/explore-education-statistics-frontend/src/modules/data-catalogue/components/Filters.tsx +++ b/src/explore-education-statistics-frontend/src/modules/data-catalogue/components/Filters.tsx @@ -160,12 +160,12 @@ export default function Filters({ { label: 'All', value: 'all' }, ...typedKeys(locationLevelsMap).map(key => { return { - label: locationLevelsMap[key].label, + label: locationLevelsMap[key].filterLabel, value: locationLevelsMap[key].code, }; }), ]} - value={geographicLevel} + value={geographicLevel ?? 'all'} order={[]} onChange={e => { onChange({ diff --git a/tests/robot-tests/tests/admin_and_public/bau/data_catalogue.robot b/tests/robot-tests/tests/admin_and_public/bau/data_catalogue.robot index d1781d21f42..ceff983c154 100644 --- a/tests/robot-tests/tests/admin_and_public/bau/data_catalogue.robot +++ b/tests/robot-tests/tests/admin_and_public/bau/data_catalogue.robot @@ -212,6 +212,22 @@ Remove release filter user checks page does not contain button ${PUPIL_ABSENCE_RELEASE_NAME} user checks selected option label id:filters-form-release All releases +Filter by geographic level + user wait for option to be available and select it css:select[id="filters-form-geographic-level"] + ... Local Authority District + + user checks page contains button Local Authority District + user checks testid element contains total-results 1 data set + +Remove geographic level filter + user clicks button Local Authority District + + user checks page does not contain button Local Authority District + user checks selected option label id:filters-form-geographic-level All + + user checks page contains button ${PUPILS_AND_SCHOOLS_THEME_TITLE} + user checks page contains button ${PUPIL_ABSENCE_PUBLICATION_TITLE} + Reset all filters user clicks element id:searchForm-search user presses keys pupil