Skip to content

Commit

Permalink
EES-5738 Changes in response to PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@mmyoungman
mmyoungman committed Dec 20, 2024
1 parent adc1eb3 commit 1d65b4b
Show file tree
Hide file tree
Showing 19 changed files with 177 additions and 116 deletions.
14 changes: 8 additions & 6 deletions ...ucationStatistics.Admin/Controllers/Api/DataSetFileGeographicLevelsMigrationController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -24,14 +25,14 @@ public class MigrationResult
}

[HttpPut("bau/migrate-datasetfile-geographiclevels")]
public async Task<MigrationResult> DataSetFileGeographicLevelsMigration(
public async Task<MigrationResult> 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)
{
Expand All @@ -53,16 +54,17 @@ public async Task<MigrationResult> 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++;
}
Expand Down
15 changes: 8 additions & 7 deletions ...aSetFileGeographicLevelsTable.Designer.cs → ...aSetFileGeographicLevelsTable.Designer.cs
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 15 additions & 7 deletions ...CreateDataSetFileGeographicLevelsTable.cs → ...CreateDataSetFileGeographicLevelsTable.cs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
using System;
using Microsoft.EntityFrameworkCore.Migrations;

#nullable disable

namespace GovUk.Education.ExploreEducationStatistics.Admin.Migrations.ContentMigrations
{
/// <inheritdoc />
Expand All @@ -10,34 +12,40 @@ public partial class EES5738_CreateDataSetFileGeographicLevelsTable : Migration
protected override void Up(MigrationBuilder migrationBuilder)
{
migrationBuilder.CreateTable(
name: "DataSetFileGeographicLevels",
name: "DataSetFileVersionGeographicLevels",
columns: table => new
{
DataSetFileVersionId = table.Column<Guid>(type: "uniqueidentifier", nullable: false),
GeographicLevel = table.Column<string>(type: "nvarchar(450)", nullable: false)
GeographicLevel = table.Column<string>(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];");
}

/// <inheritdoc />
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");
}
}
}
13 changes: 7 additions & 6 deletions ...reEducationStatistics.Admin/Migrations/ContentMigrations/ContentDbContextModelSnapshot.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Guid>("DataSetFileVersionId")
.HasColumnType("uniqueidentifier");

b.Property<string>("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 =>
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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 =>
Expand Down
22 changes: 8 additions & 14 deletions ...n.ExploreEducationStatistics.Content.Api.Tests/Controllers/DataSetFilesControllerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<ContentDbContext>(context =>
{
context.ReleaseFiles.AddRange(publication1Release1Version1Files);
context.ReleaseFiles.Add(publication1Release1Version1File);
context.ReleaseFiles.AddRange(publication2Release1Version1Files);
});

Expand All @@ -137,7 +131,7 @@ await TestApp.AddTestData<ContentDbContext>(context =>

pagedResult.AssertHasExpectedPagingAndResultCount(
expectedTotalResults: 1);
AssertResultsForExpectedReleaseFiles(publication1Release1Version1Files, pagedResult.Results);
AssertResultsForExpectedReleaseFiles([publication1Release1Version1File], pagedResult.Results);
}

[Fact]
Expand Down Expand Up @@ -1783,7 +1777,7 @@ private List<ReleaseFile> GenerateDataSetFilesForReleaseVersion(
.WithReleaseVersion(releaseVersion)
.WithFiles(_fixture.DefaultFile(FileType.Data)
.WithDataSetFileMeta(_fixture.DefaultDataSetFileMeta())
.WithDataSetFileGeographicLevels([GeographicLevel.Country])
.WithDataSetFileVersionGeographicLevels([GeographicLevel.Country])
.GenerateList(numberOfDataSets))
.GenerateList();
}
Expand Down
2 changes: 1 addition & 1 deletion ....ExploreEducationStatistics.Content.Model.Tests/Fixtures/DataImportGeneratorExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public static InstanceSetters<DataImport> SetDefaultFilesWithoutMeta(
(_, d, context) => context.Fixture
.DefaultFile(FileType.Data)
.WithDataSetFileMeta(null)
.WithDataSetFileGeographicLevels([])
.WithDataSetFileVersionGeographicLevels([])
.WithFilename($"{dataFileName}.csv")
.WithSubjectId(d.SubjectId)
)
Expand Down
50 changes: 0 additions & 50 deletions ...nStatistics.Content.Model.Tests/Fixtures/DataSetFileGeographicLevelGeneratorExtensions.cs
View file

This file was deleted.

51 changes: 51 additions & 0 deletions ...tics.Content.Model.Tests/Fixtures/DataSetFileVersionGeographicLevelGeneratorExtensions.cs
View file
Original file line number Diff line number Diff line change
@@ -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<DataSetFileVersionGeographicLevel> DefaultDataSetFileVersionGeographicLevel(this DataFixture fixture)
=> fixture.Generator<DataSetFileVersionGeographicLevel>().WithDefaults();

public static Generator<DataSetFileVersionGeographicLevel> WithDefaults(this Generator<DataSetFileVersionGeographicLevel> generator)
=> generator.ForInstance(d => d.SetDefaults());

public static InstanceSetters<DataSetFileVersionGeographicLevel> SetDefaults(
this InstanceSetters<DataSetFileVersionGeographicLevel> setters)
=> setters
.SetDefault(p => p.DataSetFileVersionId);

public static Generator<DataSetFileVersionGeographicLevel> WithDataSetFileVersion(
this Generator<DataSetFileVersionGeographicLevel> generator,
File dataSetFileVersion)
=> generator.ForInstance(s => s.SetDataSetFileVersion(dataSetFileVersion));

public static Generator<DataSetFileVersionGeographicLevel> WithDataSetFileVersionId(
this Generator<DataSetFileVersionGeographicLevel> generator,
Guid dataSetFileVersionId)
=> generator.ForInstance(s => s.SetDataSetFileVersionId(dataSetFileVersionId));

public static Generator<DataSetFileVersionGeographicLevel> WithGeographicLevel(
this Generator<DataSetFileVersionGeographicLevel> generator,
GeographicLevel geographicLevel)
=> generator.ForInstance(s => s.SetGeographicLevel(geographicLevel));

public static InstanceSetters<DataSetFileVersionGeographicLevel> SetDataSetFileVersion(
this InstanceSetters<DataSetFileVersionGeographicLevel> setters,
File dataSetFileVersion)
=> setters
.Set(f => f.DataSetFileVersion, dataSetFileVersion)
.Set(f => f.DataSetFileVersionId, dataSetFileVersion.Id);

public static InstanceSetters<DataSetFileVersionGeographicLevel> SetDataSetFileVersionId(
this InstanceSetters<DataSetFileVersionGeographicLevel> setters,
Guid dataSetFileVersionId)
=> setters.Set(f => f.DataSetFileVersionId, dataSetFileVersionId);

public static InstanceSetters<DataSetFileVersionGeographicLevel> SetGeographicLevel(
this InstanceSetters<DataSetFileVersionGeographicLevel> setters,
GeographicLevel geographicLevel)
=> setters.Set(f => f.GeographicLevel, geographicLevel);
}
Loading

0 comments on commit 1d65b4b

Please sign in to comment.