From 7e51403a74d6270cd8eb5645126f80d5b974879b Mon Sep 17 00:00:00 2001 From: jack-hive <148866614+jack-hive@users.noreply.github.com> Date: Thu, 12 Dec 2024 15:49:05 +0000 Subject: [PATCH] EES-5632 Distinguishing release redirects across different publications (#5446) * EES-5632 [WIP] Refactoring BE to return release redirects grouped by publication * EES-5632 [WIP] Integration tests * EES-5632 Enabling a different Azurite TestContainer instance to be used across each test so that the stored data doesn't interfere with other tests * EES-5632 FE - Changing the view-model field names to match the new BE field names * EES-5632 Some requested changes as per PR review * EES-5632 removing some duplicated tests * EES-5632 Reformatting tests * EES-5632 remove unused import --- .../MethodologyApprovalServiceTests.cs | 6 +- .../Methodologies/MethodologyServiceTests.cs | 42 +- .../Services/PublicationServiceTests.cs | 24 +- .../DataSetFilesControllerTests.cs | 27 +- .../Controllers/RedirectsControllerTests.cs | 522 ++++++++++++------ .../Fixtures/IntegrationTestFixture.cs | 77 ++- .../Fixtures/TestApplicationFactory.cs | 76 +-- .../MethodologyGeneratorExtensions.cs | 24 +- .../MethodologyVersionGeneratorExtensions.cs | 34 ++ .../PublicationGeneratorExtensions.cs | 43 ++ .../Fixtures/ReleaseGeneratorExtensions.cs | 43 ++ .../MethodologyVersion.cs | 2 + .../Publication.cs | 2 + .../PublicationRedirect.cs | 8 +- .../Release.cs | 2 + .../ReleaseRedirect.cs | 6 +- .../RedirectsServiceTests.cs | 142 +---- .../RedirectsService.cs | 62 ++- .../RedirectViewModels.cs | 6 +- .../pages/__tests__/redirectPages.test.ts | 4 +- .../src/middleware/pages/redirectPages.ts | 4 +- .../src/services/redirectService.ts | 4 +- 22 files changed, 680 insertions(+), 480 deletions(-) diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/Methodologies/MethodologyApprovalServiceTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/Methodologies/MethodologyApprovalServiceTests.cs index 0105556c036..351828f7342 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/Methodologies/MethodologyApprovalServiceTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/Methodologies/MethodologyApprovalServiceTests.cs @@ -681,9 +681,9 @@ public async Task UpdateApprovalStatus_ApprovingUsingImmediateStrategy() redirectsCacheService.Setup(mock => mock.UpdateRedirects()) .ReturnsAsync(new RedirectsViewModel( - Publications: [], - Methodologies: [], - Releases: [])); + PublicationRedirects: [], + MethodologyRedirects: [], + ReleaseRedirectsByPublicationSlug: [])); await using (var context = InMemoryApplicationDbContext(contentDbContextId)) { diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/Methodologies/MethodologyServiceTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/Methodologies/MethodologyServiceTests.cs index 17499e9130b..8c4afcd5a6e 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/Methodologies/MethodologyServiceTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/Methodologies/MethodologyServiceTests.cs @@ -3600,9 +3600,9 @@ public async Task PublicationTitleOrSlugChanged() var redirectsCacheService = new Mock(MockBehavior.Strict); redirectsCacheService.Setup(mock => mock.UpdateRedirects()) .ReturnsAsync(new RedirectsViewModel( - Publications: [], - Methodologies: [], - Releases: [])); + PublicationRedirects: [], + MethodologyRedirects: [], + ReleaseRedirectsByPublicationSlug: [])); var service = SetupMethodologyService(contentDbContext, redirectsCacheService: redirectsCacheService.Object); @@ -3662,9 +3662,9 @@ public async Task PublicationTitleOrSlugChanged_NoMethodologyRedirectAsMethodolo var redirectsCacheService = new Mock(MockBehavior.Strict); redirectsCacheService.Setup(mock => mock.UpdateRedirects()) .ReturnsAsync(new RedirectsViewModel( - Publications: [], - Methodologies: [], - Releases: [])); + PublicationRedirects: [], + MethodologyRedirects: [], + ReleaseRedirectsByPublicationSlug: [])); var service = SetupMethodologyService(contentDbContext, redirectsCacheService: redirectsCacheService.Object); @@ -3745,9 +3745,9 @@ await contentDbContext.PublicationMethodologies.AddRangeAsync( var redirectsCacheService = new Mock(MockBehavior.Strict); redirectsCacheService.Setup(mock => mock.UpdateRedirects()) .ReturnsAsync(new RedirectsViewModel( - Publications: [], - Methodologies: [], - Releases: [])); + PublicationRedirects: [], + MethodologyRedirects: [], + ReleaseRedirectsByPublicationSlug: [])); var service = SetupMethodologyService(contentDbContext, redirectsCacheService: redirectsCacheService.Object); @@ -3847,9 +3847,9 @@ public async Task PublicationTitleOrSlugChanged_MethodologySlugIsAlternativeSlug var redirectsCacheService = new Mock(MockBehavior.Strict); redirectsCacheService.Setup(mock => mock.UpdateRedirects()) .ReturnsAsync(new RedirectsViewModel( - Publications: [], - Methodologies: [], - Releases: [])); + PublicationRedirects: [], + MethodologyRedirects: [], + ReleaseRedirectsByPublicationSlug: [])); var service = SetupMethodologyService(contentDbContext, redirectsCacheService: redirectsCacheService.Object); @@ -3917,9 +3917,9 @@ public async Task PublicationTitleOrSlugChanged_MethodologyIsLive() var redirectsCacheService = new Mock(MockBehavior.Strict); redirectsCacheService.Setup(mock => mock.UpdateRedirects()) .ReturnsAsync(new RedirectsViewModel( - Publications: [], - Methodologies: [], - Releases: [])); + PublicationRedirects: [], + MethodologyRedirects: [], + ReleaseRedirectsByPublicationSlug: [])); var service = SetupMethodologyService(contentDbContext, redirectsCacheService: redirectsCacheService.Object); @@ -4004,9 +4004,9 @@ public async Task var redirectsCacheService = new Mock(MockBehavior.Strict); redirectsCacheService.Setup(mock => mock.UpdateRedirects()) .ReturnsAsync(new RedirectsViewModel( - Publications: [], - Methodologies: [], - Releases: [])); + PublicationRedirects: [], + MethodologyRedirects: [], + ReleaseRedirectsByPublicationSlug: [])); var service = SetupMethodologyService(contentDbContext, redirectsCacheService: redirectsCacheService.Object); @@ -4110,9 +4110,9 @@ public async Task var redirectsCacheService = new Mock(MockBehavior.Strict); redirectsCacheService.Setup(mock => mock.UpdateRedirects()) .ReturnsAsync(new RedirectsViewModel( - Publications: [], - Methodologies: [], - Releases: [])); + PublicationRedirects: [], + MethodologyRedirects: [], + ReleaseRedirectsByPublicationSlug: [])); var service = SetupMethodologyService(contentDbContext, redirectsCacheService: redirectsCacheService.Object); diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/PublicationServiceTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/PublicationServiceTests.cs index 4841cb8ce96..39fb7872c3f 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/PublicationServiceTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/PublicationServiceTests.cs @@ -1127,9 +1127,9 @@ public async Task UpdatePublication_AlreadyPublished() redirectsCacheService.Setup(mock => mock.UpdateRedirects()) .ReturnsAsync(new RedirectsViewModel( - Publications: [], - Methodologies: [], - Releases: [])); + PublicationRedirects: [], + MethodologyRedirects: [], + ReleaseRedirectsByPublicationSlug: [])); var publicationService = BuildPublicationService(context, methodologyService: methodologyService.Object, @@ -1272,9 +1272,9 @@ public async Task UpdatePublication_TitleChangesPublicationAndMethodologySlug() redirectsCacheService.Setup(mock => mock.UpdateRedirects()) .ReturnsAsync(new RedirectsViewModel( - Publications: [], - Methodologies: [], - Releases: [])); + PublicationRedirects: [], + MethodologyRedirects: [], + ReleaseRedirectsByPublicationSlug: [])); var publicationService = BuildPublicationService(context, methodologyService: methodologyService.Object, @@ -1579,9 +1579,9 @@ public async Task UpdatePublication_CreateRedirectIfLiveSlugChanged() var redirectsCacheService = new Mock(Strict); redirectsCacheService.Setup(mock => mock.UpdateRedirects()) .ReturnsAsync(new RedirectsViewModel( - Publications: [], - Methodologies: [], - Releases: [])); + PublicationRedirects: [], + MethodologyRedirects: [], + ReleaseRedirectsByPublicationSlug: [])); var publicationService = BuildPublicationService(context, methodologyService: methodologyService.Object, @@ -1678,9 +1678,9 @@ public async Task UpdatePublication_ChangeBackToPreviousLiveSlug() var redirectsCacheService = new Mock(Strict); redirectsCacheService.Setup(mock => mock.UpdateRedirects()) .ReturnsAsync(new RedirectsViewModel( - Publications: [], - Methodologies: [], - Releases: [])); + PublicationRedirects: [], + MethodologyRedirects: [], + ReleaseRedirectsByPublicationSlug: [])); var publicationService = BuildPublicationService(context, methodologyService: methodologyService.Object, 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 1de823466ab..91a6b434769 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.Api.Tests/Controllers/DataSetFilesControllerTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Api.Tests/Controllers/DataSetFilesControllerTests.cs @@ -1759,7 +1759,7 @@ private static Mock ContentDbContextMock( public class DownloadDataSetFileTests(TestApplicationFactory testApp) : DataSetFilesControllerTests(testApp) { - public override async Task InitializeAsync() => await TestApp.StartAzurite(); + public override async Task InitializeAsync() => await StartAzurite(); [Fact] public async Task DownloadDataSetFile() @@ -1844,7 +1844,7 @@ public async Task DownloadDataSetFile_NotPublished() .WithReleaseVersion(publication.ReleaseVersions[0]) .WithFile(_fixture.DefaultFile(FileType.Data)); - await TestApp.StartAzurite(); + await StartAzurite(); var testApp = BuildApp(enableAzurite: true); var publicBlobStorageService = testApp.Services.GetRequiredService(); @@ -1871,7 +1871,7 @@ await TestApp.AddTestData(context => public class ListSitemapItemsTests(TestApplicationFactory testApp) : DataSetFilesControllerTests(testApp) { - public override async Task InitializeAsync() => await TestApp.StartAzurite(); + public override async Task InitializeAsync() => await StartAzurite(); private async Task InvokeListSitemapItems( WebApplicationFactory? app = null) @@ -1904,7 +1904,7 @@ public async Task ListSitemapItems() )) ); - await TestApp.StartAzurite(); + await StartAzurite(); var testApp = BuildApp(enableAzurite: true); var publicBlobStorageService = testApp.Services.GetRequiredService(); @@ -1944,7 +1944,7 @@ public class GetDataSetFileTests(TestApplicationFactory testApp) : DataSetFilesC { public override async Task InitializeAsync() { - await TestApp.StartAzurite(); + await StartAzurite(); } [Fact] @@ -1975,7 +1975,7 @@ await TestApp.AddTestData(context => context.ReleaseFiles.Add(releaseFile); }); - await TestApp.StartAzurite(); + await StartAzurite(); var testApp = BuildApp(enableAzurite: true); @@ -2098,7 +2098,7 @@ await TestApp.AddTestData(context => context.ReleaseFiles.Add(releaseFile); }); - await TestApp.StartAzurite(); + await StartAzurite(); var testApp = BuildApp(enableAzurite: true); @@ -2163,7 +2163,7 @@ await TestApp.AddTestData(context => context.ReleaseFiles.Add(releaseFile); }); - await TestApp.StartAzurite(); + await StartAzurite(); var testApp = BuildApp(enableAzurite: true); @@ -2224,7 +2224,7 @@ await TestApp.AddTestData(context => context.ReleaseFiles.Add(releaseFile); }); - await TestApp.StartAzurite(); + await StartAzurite(); var testApp = BuildApp(enableAzurite: true); @@ -2281,7 +2281,7 @@ await TestApp.AddTestData(context => context.ReleaseFiles.Add(releaseFile); }); - await TestApp.StartAzurite(); + await StartAzurite(); var testApp = BuildApp(enableAzurite: true); @@ -2362,7 +2362,7 @@ await TestApp.AddTestData(context => context.ReleaseFootnote.AddRange(releaseFootnote1, releaseFootnote2); }); - await TestApp.StartAzurite(); + await StartAzurite(); var testApp = BuildApp(enableAzurite: true); @@ -2466,7 +2466,7 @@ await TestApp.AddTestData(context => context.ReleaseFiles.AddRange(releaseFile0, releaseFile1, releaseFile2); }); - await TestApp.StartAzurite(); + await StartAzurite(); var testApp = BuildApp(enableAzurite: true); @@ -2532,8 +2532,7 @@ private WebApplicationFactory BuildApp( StatisticsDbContext? statisticsDbContext = null, bool enableAzurite = false) { - return TestApp - .WithAzurite(enabled: enableAzurite) + return WithAzurite(enabled: enableAzurite) .ConfigureServices(services => { services.ReplaceService(MemoryCacheService); diff --git a/src/GovUk.Education.ExploreEducationStatistics.Content.Api.Tests/Controllers/RedirectsControllerTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Content.Api.Tests/Controllers/RedirectsControllerTests.cs index 7577fb5075c..f2336a62773 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.Api.Tests/Controllers/RedirectsControllerTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Api.Tests/Controllers/RedirectsControllerTests.cs @@ -20,189 +20,388 @@ namespace GovUk.Education.ExploreEducationStatistics.Content.Api.Tests.Controlle public abstract class RedirectsControllerTests(TestApplicationFactory testApp) : IntegrationTestFixture(testApp) { - public class ListTests(TestApplicationFactory testApp) : RedirectsControllerTests(testApp) + public abstract class ListTests(TestApplicationFactory testApp) : RedirectsControllerTests(testApp) { - public override async Task InitializeAsync() => await TestApp.StartAzurite(); + public override async Task InitializeAsync() => await StartAzurite(); - [Fact] - public async Task RedirectsExist_Returns200WithRedirects() + public class GeneralTests(TestApplicationFactory testApp) : ListTests(testApp) { - var publicationRedirects = DataFixture.DefaultPublicationRedirect() - .WithPublication(DataFixture.DefaultPublication()) - .GenerateList(3); - - var releaseRedirects = DataFixture.DefaultReleaseRedirect() - .WithRelease(DataFixture.DefaultRelease()) - .GenerateList(3); - - var methodologyVersions = DataFixture.DefaultMethodologyVersion() - .ForIndex(3, s => s.SetPublished(DateTime.UtcNow)) - .GenerateList(4); + [Fact] + public async Task RedirectsExistInCache_Returns200WithRedirects() + { + var app = BuildApp(enableAzurite: true); + var client = app.CreateClient(); + + var blobCacheService = app.Services.GetRequiredService(); + + List cachedMethodologyRedirects = + [ + new(FromSlug: "original-methodology-slug-1", ToSlug: "updated-methodology-slug-1"), + new(FromSlug: "original-methodology-slug-2", ToSlug: "updated-methodology-slug-2"), + ]; + + List cachedPublicationRedirects = + [ + new(FromSlug: "original-publication-slug-1", ToSlug: "updated-publication-slug-1"), + new(FromSlug: "original-publication-slug-2", ToSlug: "updated-publication-slug-2"), + ]; + + var cachedReleaseRedirectsByPublicationSlug = new Dictionary> + { + { + "updated-publication-slug-1", + new List() + { + new(FromSlug: "original-release-slug-1", ToSlug: "updated-release-slug-1"), + new(FromSlug: "original-release-slug-2", ToSlug: "updated-release-slug-2"), + } + } + }; + + var cachedViewModel = new RedirectsViewModel( + PublicationRedirects: cachedPublicationRedirects, + ReleaseRedirectsByPublicationSlug: cachedReleaseRedirectsByPublicationSlug, + MethodologyRedirects: cachedMethodologyRedirects); + + await blobCacheService.SetItemAsync(new RedirectsCacheKey(), cachedViewModel); + + var response = await ListRedirects(client); + + var viewModel = response.AssertOk(); + + Assert.Equal(cachedMethodologyRedirects.Count, viewModel.MethodologyRedirects.Count); + Assert.All( + cachedMethodologyRedirects, + cmr => Assert.Contains( + viewModel.MethodologyRedirects, + rvm => cmr.FromSlug == rvm.FromSlug && cmr.ToSlug == rvm.ToSlug)); + + Assert.Equal(cachedPublicationRedirects.Count, viewModel.PublicationRedirects.Count); + Assert.All( + cachedPublicationRedirects, + cpr => Assert.Contains( + viewModel.PublicationRedirects, + rvm => cpr.FromSlug == rvm.FromSlug && cpr.ToSlug == rvm.ToSlug)); + + var cachedReleaseRedirectsForPublication = Assert.Single(cachedReleaseRedirectsByPublicationSlug); + Assert.Equal(cachedReleaseRedirectsForPublication.Value.Count, + viewModel.ReleaseRedirectsByPublicationSlug["updated-publication-slug-1"].Count); + Assert.Equal("updated-publication-slug-1", cachedReleaseRedirectsForPublication.Key); + Assert.All( + cachedReleaseRedirectsForPublication.Value, + crr => Assert.Contains( + viewModel.ReleaseRedirectsByPublicationSlug["updated-publication-slug-1"], + rvm => crr.FromSlug == rvm.FromSlug && crr.ToSlug == rvm.ToSlug)); + } + + [Fact] + public async Task NoRedirectsExist_Returns200WithNoRedirects() + { + var client = BuildApp(enableAzurite: true).CreateClient(); + var response = await ListRedirects(client); - Methodology methodology = DataFixture.DefaultMethodology() - .WithMethodologyVersions(methodologyVersions) - .WithLatestPublishedVersion(methodologyVersions.Last()); + var viewModel = response.AssertOk(); - var methodologyRedirects = DataFixture.DefaultMethodologyRedirect() - .ForIndex(0, s => s.SetMethodologyVersion(methodologyVersions[0])) - .ForIndex(1, s => s.SetMethodologyVersion(methodologyVersions[1])) - .ForIndex(2, s => s.SetMethodologyVersion(methodologyVersions[2])) - .GenerateList(3); + Assert.Empty(viewModel.PublicationRedirects); + Assert.Empty(viewModel.ReleaseRedirectsByPublicationSlug); + Assert.Empty(viewModel.MethodologyRedirects); + } + } - await TestApp.AddTestData(context => + public class MethodologyRedirectsTests(TestApplicationFactory testApp) : ListTests(testApp) + { + [Fact] + public async Task RedirectsExist_Returns200WithRedirects() { - context.PublicationRedirects.AddRange(publicationRedirects); - context.ReleaseRedirects.AddRange(releaseRedirects); - context.MethodologyRedirects.AddRange(methodologyRedirects); - }); - - var response = await ListRedirects(); - - var viewModel = response.AssertOk(); - - Assert.All( - publicationRedirects, - pr => Assert.Contains( - viewModel.Publications, - rvm => pr.Slug == rvm.FromSlug && pr.Publication.Slug == rvm.ToSlug)); - Assert.All( - releaseRedirects, - rr => Assert.Contains( - viewModel.Releases, - rvm => rr.Slug == rvm.FromSlug && rr.Release.Slug == rvm.ToSlug)); - Assert.All( - methodologyRedirects, - mr => Assert.Contains( - viewModel.Methodologies, - rvm => mr.Slug == rvm.FromSlug && mr.MethodologyVersion.Methodology.OwningPublicationSlug == rvm.ToSlug)); + Methodology methodology = DataFixture.DefaultMethodology() + .WithMethodologyVersions( + DataFixture.DefaultMethodologyVersion() + .WithRedirects([DataFixture.DefaultMethodologyRedirect()]) + .ForIndex(0, s => s.SetRedirects([DataFixture.DefaultMethodologyRedirect()])) + .ForIndex(1, + s => s + .SetRedirects([DataFixture.DefaultMethodologyRedirect()]) + .SetPublished(DateTime.UtcNow)) + .GenerateList(2)); + + await TestApp.AddTestData(context => context.Methodologies.Add(methodology)); + + var client = BuildApp(enableAzurite: true).CreateClient(); + var response = await ListRedirects(client); + + var viewModel = response.AssertOk(); + + var redirects = methodology.Versions + .SelectMany(mv => mv.MethodologyRedirects) + .ToList(); + + Assert.Equal(redirects.Count, viewModel.MethodologyRedirects.Count); + Assert.All( + redirects, + mr => Assert.Contains( + viewModel.MethodologyRedirects, + rvm => + mr.Slug == rvm.FromSlug && + mr.MethodologyVersion.Methodology.OwningPublicationSlug == rvm.ToSlug)); + } + + [Fact] + public async Task RedirectsExist_RedirectsAreCached() + { + Methodology methodology = DataFixture.DefaultMethodology() + .WithMethodologyVersions( + DataFixture.DefaultMethodologyVersion() + .WithRedirects([DataFixture.DefaultMethodologyRedirect()]) + .ForIndex(0, s => s.SetRedirects([DataFixture.DefaultMethodologyRedirect()])) + .ForIndex(1, + s => s + .SetRedirects([DataFixture.DefaultMethodologyRedirect()]) + .SetPublished(DateTime.UtcNow)) + .GenerateList(2)); + + await TestApp.AddTestData(context => context.Methodologies.Add(methodology)); + + var app = BuildApp(enableAzurite: true); + var client = app.CreateClient(); + + await ListRedirects(client); + + var blobCacheService = app.Services.GetRequiredService(); + + var cachedValue = + await blobCacheService.GetItemAsync(new RedirectsCacheKey(), typeof(RedirectsViewModel)); + var cachedRedirectsViewModel = Assert.IsType(cachedValue); + + var redirects = methodology.Versions + .SelectMany(mv => mv.MethodologyRedirects) + .ToList(); + + Assert.Empty(cachedRedirectsViewModel.PublicationRedirects); + Assert.Empty(cachedRedirectsViewModel.ReleaseRedirectsByPublicationSlug); + + Assert.Equal(redirects.Count, cachedRedirectsViewModel.MethodologyRedirects.Count); + Assert.All( + redirects, + mr => Assert.Contains( + cachedRedirectsViewModel.MethodologyRedirects, + rvm => + mr.Slug == rvm.FromSlug && + mr.MethodologyVersion.Methodology.OwningPublicationSlug == rvm.ToSlug)); + } } - [Fact] - public async Task RedirectsExist_RedirectsAreCached() + public class PublicationRedirectsTests(TestApplicationFactory testApp) : ListTests(testApp) { - var publicationRedirects = DataFixture.DefaultPublicationRedirect() - .WithPublication(DataFixture.DefaultPublication()) - .GenerateList(3); + [Fact] + public async Task RedirectsExist_Returns200WithRedirects() + { + var publicationRedirects = DataFixture.DefaultPublicationRedirect() + .WithPublication(DataFixture.DefaultPublication()) + .GenerateList(2); + + await TestApp.AddTestData(context => + context.PublicationRedirects.AddRange(publicationRedirects)); + + var client = BuildApp(enableAzurite: true).CreateClient(); + var response = await ListRedirects(client); + + var viewModel = response.AssertOk(); + + Assert.Equal(publicationRedirects.Count, viewModel.PublicationRedirects.Count); + Assert.All( + publicationRedirects, + pr => Assert.Contains( + viewModel.PublicationRedirects, + rvm => + pr.Slug == rvm.FromSlug && + pr.Publication.Slug == rvm.ToSlug)); + } + + [Fact] + public async Task RedirectsExist_RedirectsAreCached() + { + var publicationRedirects = DataFixture.DefaultPublicationRedirect() + .WithPublication(DataFixture.DefaultPublication()) + .GenerateList(2); - var releaseRedirects = DataFixture.DefaultReleaseRedirect() - .WithRelease(DataFixture.DefaultRelease()) - .GenerateList(3); + await TestApp.AddTestData(context => + context.PublicationRedirects.AddRange(publicationRedirects)); - var methodologyVersions = DataFixture.DefaultMethodologyVersion() - .ForIndex(3, s => s.SetPublished(DateTime.UtcNow)) - .GenerateList(4); + await StartAzurite(); - Methodology methodology = DataFixture.DefaultMethodology() - .WithMethodologyVersions(methodologyVersions) - .WithLatestPublishedVersion(methodologyVersions.Last()); + var app = BuildApp(enableAzurite: true); + var client = app.CreateClient(); - var methodologyRedirects = DataFixture.DefaultMethodologyRedirect() - .ForIndex(0, s => s.SetMethodologyVersion(methodologyVersions[0])) - .ForIndex(1, s => s.SetMethodologyVersion(methodologyVersions[1])) - .ForIndex(2, s => s.SetMethodologyVersion(methodologyVersions[2])) - .GenerateList(3); + await ListRedirects(client); - await TestApp.AddTestData(context => - { - context.PublicationRedirects.AddRange(publicationRedirects); - context.ReleaseRedirects.AddRange(releaseRedirects); - context.MethodologyRedirects.AddRange(methodologyRedirects); - }); - - var app = BuildApp(); - var client = app.CreateClient(); - - await ListRedirects(client); - - var blobCacheService = app.Services.GetRequiredService(); - - var cachedValue = await blobCacheService.GetItemAsync(new RedirectsCacheKey(), typeof(RedirectsViewModel)); - var cachedRedirectsViewModel = Assert.IsType(cachedValue); - - Assert.All( - publicationRedirects, - pr => Assert.Contains( - cachedRedirectsViewModel.Publications, - rvm => pr.Slug == rvm.FromSlug && pr.Publication.Slug == rvm.ToSlug)); - Assert.All( - releaseRedirects, - rr => Assert.Contains( - cachedRedirectsViewModel.Releases, - rvm => rr.Slug == rvm.FromSlug && rr.Release.Slug == rvm.ToSlug)); - Assert.All( - methodologyRedirects, - mr => Assert.Contains( - cachedRedirectsViewModel.Methodologies, - rvm => mr.Slug == rvm.FromSlug && mr.MethodologyVersion.Methodology.OwningPublicationSlug == rvm.ToSlug)); - } + var blobCacheService = app.Services.GetRequiredService(); - [Fact] - public async Task RedirectsExistInCache_Returns200WithRedirects() - { - var app = BuildApp(); - var client = app.CreateClient(); + var cachedValue = + await blobCacheService.GetItemAsync(new RedirectsCacheKey(), typeof(RedirectsViewModel)); + var cachedRedirectsViewModel = Assert.IsType(cachedValue); - var blobCacheService = app.Services.GetRequiredService(); + Assert.Empty(cachedRedirectsViewModel.MethodologyRedirects); + Assert.Empty(cachedRedirectsViewModel.ReleaseRedirectsByPublicationSlug); - var cachedPublicationRedirects = new List() - { - new(FromSlug: "publication_fromSlug_1", ToSlug: "publication_toSlug_1"), - new(FromSlug: "publication_fromSlug_2", ToSlug: "publication_toSlug_2"), - new(FromSlug: "publication_fromSlug_3", ToSlug: "publication_toSlug_3") - }; + Assert.Equal(publicationRedirects.Count, cachedRedirectsViewModel.PublicationRedirects.Count); + Assert.All( + publicationRedirects, + pr => Assert.Contains( + cachedRedirectsViewModel.PublicationRedirects, + rvm => + pr.Slug == rvm.FromSlug && + pr.Publication.Slug == rvm.ToSlug)); + } + } - var cachedReleaseRedirects = new List() + public class ReleaseRedirectsTests(TestApplicationFactory testApp) : ListTests(testApp) + { + [Fact] + public async Task ReleaseRedirectDoesNotExistForPublicationWithRedirect_Returns200WithRedirects() { - new(FromSlug: "release_fromSlug_1", ToSlug: "release_toSlug_1"), - new(FromSlug: "release_fromSlug_2", ToSlug: "release_toSlug_2"), - new(FromSlug: "release_fromSlug_3", ToSlug: "release_toSlug_3") - }; + PublicationRedirect publicationRedirect = DataFixture.DefaultPublicationRedirect() + .WithPublication(DataFixture.DefaultPublication()); - var cachedMethodologyRedirects = new List() - { - new(FromSlug: "methodology_fromSlug_1", ToSlug: "methodology_toSlug_1"), - new(FromSlug: "methodology_fromSlug_2", ToSlug: "methodology_toSlug_2"), - new(FromSlug: "methodology_fromSlug_3", ToSlug: "methodology_toSlug_3") - }; - - var cachedViewModel = new RedirectsViewModel( - Publications: cachedPublicationRedirects, - Releases: cachedReleaseRedirects, - Methodologies: cachedMethodologyRedirects); - - await blobCacheService.SetItemAsync(new RedirectsCacheKey(), cachedViewModel); - - var response = await ListRedirects(client); - - var viewModel = response.AssertOk(); - - Assert.All( - cachedPublicationRedirects, - cpr => Assert.Contains( - viewModel.Publications, - rvm => cpr.FromSlug == rvm.FromSlug && cpr.ToSlug == rvm.ToSlug)); - Assert.All( - cachedReleaseRedirects, - crr => Assert.Contains( - viewModel.Releases, - rvm => crr.FromSlug == rvm.FromSlug && crr.ToSlug == rvm.ToSlug)); - Assert.All( - cachedMethodologyRedirects, - cmr => Assert.Contains( - viewModel.Methodologies, - rvm => cmr.FromSlug == rvm.FromSlug && cmr.ToSlug == rvm.ToSlug)); - } + await TestApp.AddTestData(context => + context.PublicationRedirects.Add(publicationRedirect)); - [Fact] - public async Task NoRedirectsExist_Returns200WithNoRedirects() - { - var response = await ListRedirects(); + var client = BuildApp(enableAzurite: true).CreateClient(); + var response = await ListRedirects(client); - var viewModel = response.AssertOk(); + var viewModel = response.AssertOk(); - Assert.Empty(viewModel.Publications); - Assert.Empty(viewModel.Releases); - Assert.Empty(viewModel.Methodologies); + var publicationRedirectViewModel = Assert.Single(viewModel.PublicationRedirects); + Assert.Equal(publicationRedirect.Slug, publicationRedirectViewModel.FromSlug); + Assert.Equal(publicationRedirect.Publication.Slug, publicationRedirectViewModel.ToSlug); + + Assert.Empty(viewModel.ReleaseRedirectsByPublicationSlug); + } + + [Fact] + public async Task ReleaseRedirectExistsForPublicationWithRedirect_Returns200WithRedirects() + { + Publication publication = DataFixture.DefaultPublication() + .WithReleases( + [ + DataFixture + .DefaultRelease(publishedVersions: 1) + .WithSlug("updated-release-slug-1") + .WithRedirects( + DataFixture.DefaultReleaseRedirect() + .ForIndex(0, s => s.SetSlug("first-release-slug-1")) + .ForIndex(1, s => s.SetSlug("second-release-slug-1")) + .GenerateList(2) + ) + ]) + .WithSlug("updated-publication-slug-1") + .WithRedirects( + [ + DataFixture.DefaultPublicationRedirect() + .WithSlug("original-publication-slug-1") + ]); + + await TestApp.AddTestData(context => context.Publications.Add(publication)); + + var client = BuildApp(enableAzurite: true).CreateClient(); + var response = await ListRedirects(client); + + var viewModel = response.AssertOk(); + + var publicationRedirectViewModel = Assert.Single(viewModel.PublicationRedirects); + Assert.Equal("original-publication-slug-1", publicationRedirectViewModel.FromSlug); + Assert.Equal("updated-publication-slug-1", publicationRedirectViewModel.ToSlug); + + var releaseRedirectsForPublication = Assert.Single(viewModel.ReleaseRedirectsByPublicationSlug); + Assert.Equal("updated-publication-slug-1", releaseRedirectsForPublication.Key); + + Assert.Equal(2, releaseRedirectsForPublication.Value.Count); + Assert.Contains(releaseRedirectsForPublication.Value, + r => r.FromSlug == "first-release-slug-1" && r.ToSlug == "updated-release-slug-1"); + Assert.Contains(releaseRedirectsForPublication.Value, + r => r.FromSlug == "second-release-slug-1" && r.ToSlug == "updated-release-slug-1"); + } + + [Fact] + public async Task ReleaseRedirectExistsForPublicationWithoutRedirect_Returns200WithRedirects() + { + Publication publication = DataFixture.DefaultPublication() + .WithReleases( + [ + DataFixture + .DefaultRelease(publishedVersions: 1) + .WithSlug("updated-release-slug-1") + .WithRedirects( + DataFixture.DefaultReleaseRedirect() + .ForIndex(0, s => s.SetSlug("first-release-slug-1")) + .ForIndex(1, s => s.SetSlug("second-release-slug-1")) + .GenerateList(2) + ) + ]) + .WithSlug("original-publication-slug-1"); + + await TestApp.AddTestData(context => context.Publications.Add(publication)); + + var client = BuildApp(enableAzurite: true).CreateClient(); + var response = await ListRedirects(client); + + var viewModel = response.AssertOk(); + + Assert.Empty(viewModel.PublicationRedirects); + + var releaseRedirectsForPublication = Assert.Single(viewModel.ReleaseRedirectsByPublicationSlug); + Assert.Equal("original-publication-slug-1", releaseRedirectsForPublication.Key); + + Assert.Equal(2, releaseRedirectsForPublication.Value.Count); + Assert.Contains(releaseRedirectsForPublication.Value, + r => r.FromSlug == "first-release-slug-1" && r.ToSlug == "updated-release-slug-1"); + Assert.Contains(releaseRedirectsForPublication.Value, + r => r.FromSlug == "second-release-slug-1" && r.ToSlug == "updated-release-slug-1"); + } + + [Fact] + public async Task RedirectsExist_RedirectsAreCached() + { + Publication publication = DataFixture.DefaultPublication() + .WithReleases( + [ + DataFixture + .DefaultRelease(publishedVersions: 1) + .WithSlug("updated-release-slug-1") + .WithRedirects( + DataFixture.DefaultReleaseRedirect() + .ForIndex(0, s => s.SetSlug("first-release-slug-1")) + .ForIndex(1, s => s.SetSlug("second-release-slug-1")) + .GenerateList(2) + ) + ]) + .WithSlug("original-publication-slug-1"); + + await TestApp.AddTestData(context => context.Publications.Add(publication)); + + var app = BuildApp(enableAzurite: true); + var client = app.CreateClient(); + + await ListRedirects(client); + + var blobCacheService = app.Services.GetRequiredService(); + + var cachedValue = + await blobCacheService.GetItemAsync(new RedirectsCacheKey(), typeof(RedirectsViewModel)); + var cachedRedirectsViewModel = Assert.IsType(cachedValue); + + Assert.Empty(cachedRedirectsViewModel.MethodologyRedirects); + Assert.Empty(cachedRedirectsViewModel.PublicationRedirects); + + var releaseRedirectsForPublication = + Assert.Single(cachedRedirectsViewModel.ReleaseRedirectsByPublicationSlug); + Assert.Equal("original-publication-slug-1", releaseRedirectsForPublication.Key); + + Assert.Equal(2, releaseRedirectsForPublication.Value.Count); + Assert.Contains(releaseRedirectsForPublication.Value, + r => r.FromSlug == "first-release-slug-1" && r.ToSlug == "updated-release-slug-1"); + Assert.Contains(releaseRedirectsForPublication.Value, + r => r.FromSlug == "second-release-slug-1" && r.ToSlug == "updated-release-slug-1"); + } } private async Task ListRedirects( @@ -214,9 +413,8 @@ private async Task ListRedirects( } } - private WebApplicationFactory BuildApp() + private WebApplicationFactory BuildApp(bool enableAzurite = false) { - return TestApp - .WithAzurite(enabled: true); + return WithAzurite(enabled: enableAzurite); } } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Content.Api.Tests/Fixtures/IntegrationTestFixture.cs b/src/GovUk.Education.ExploreEducationStatistics.Content.Api.Tests/Fixtures/IntegrationTestFixture.cs index 8590f56d116..ad9485636ad 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.Api.Tests/Fixtures/IntegrationTestFixture.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Api.Tests/Fixtures/IntegrationTestFixture.cs @@ -1,25 +1,50 @@ #nullable enable +using System; +using System.Collections.Generic; using System.Threading.Tasks; +using DotNet.Testcontainers.Containers; +using GovUk.Education.ExploreEducationStatistics.Common.Services.Interfaces; using GovUk.Education.ExploreEducationStatistics.Common.Tests.Fixtures; -using GovUk.Education.ExploreEducationStatistics.Content.Model.Database; -using GovUk.Education.ExploreEducationStatistics.Data.Model.Database; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.Extensions.Configuration; +using Testcontainers.Azurite; using Xunit; +using GovUk.Education.ExploreEducationStatistics.Common.Services; +using GovUk.Education.ExploreEducationStatistics.Common.Tests.Extensions; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; namespace GovUk.Education.ExploreEducationStatistics.Content.Api.Tests.Fixtures; [Collection(CacheTestFixture.CollectionName)] -public abstract class IntegrationTestFixture : +public abstract class IntegrationTestFixture(TestApplicationFactory testApp) : CacheServiceTestFixture, IClassFixture, IAsyncLifetime { + private readonly AzuriteContainer _azuriteContainer = new AzuriteBuilder() + .WithImage("mcr.microsoft.com/azure-storage/azurite:3.31.0") + .Build(); + protected readonly DataFixture DataFixture = new(); - protected readonly TestApplicationFactory TestApp; + protected readonly TestApplicationFactory TestApp = testApp; - internal IntegrationTestFixture(TestApplicationFactory testApp) + /// + /// Start the Azurite container. Once started, the test app must also + /// be configured with to use it. + /// + /// + /// We don't start the Azurite container in a class fixture as there currently + /// isn't a good way to clear it after each test. The current approach is to + /// restart the container for each test case (which is quite slow). + /// See: https://github.com/Azure/Azurite/issues/588. + /// For now, we should manually control the Azurite container's lifecycle by + /// calling this on a case-by-case basis. + /// + public async Task StartAzurite() { - TestApp = testApp; + await _azuriteContainer.StartAsync(); } public virtual async Task InitializeAsync() @@ -29,8 +54,42 @@ public virtual async Task InitializeAsync() public virtual async Task DisposeAsync() { - await TestApp.EnsureDatabaseDeleted(); - await TestApp.EnsureDatabaseDeleted(); - await TestApp.StopAzurite(); + await TestApp.ClearAllTestData(); + await _azuriteContainer.DisposeAsync(); + } + + public WebApplicationFactory WithAzurite(bool enabled = true) + { + if (!enabled) + { + return TestApp; + } + + if (_azuriteContainer.State != TestcontainersStates.Running) + { + throw new InvalidOperationException( + $"Azurite container must be started via '{nameof(StartAzurite)}' method first"); + } + + return TestApp.WithWebHostBuilder(builder => + { + builder + .ConfigureAppConfiguration((_, config) => + { + config.AddInMemoryCollection( + [ + new KeyValuePair("PublicStorage", _azuriteContainer.GetConnectionString()) + ]); + }) + .ConfigureServices(services => + { + services.ReplaceService(sp => + new PublicBlobStorageService( + _azuriteContainer.GetConnectionString(), + sp.GetRequiredService>() + ) + ); + }); + }); } } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Content.Api.Tests/Fixtures/TestApplicationFactory.cs b/src/GovUk.Education.ExploreEducationStatistics.Content.Api.Tests/Fixtures/TestApplicationFactory.cs index 2b7a6016445..8e430dc34c6 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.Api.Tests/Fixtures/TestApplicationFactory.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Api.Tests/Fixtures/TestApplicationFactory.cs @@ -1,91 +1,21 @@ #nullable enable -using System; -using System.Collections.Generic; using System.Threading.Tasks; -using DotNet.Testcontainers.Containers; -using GovUk.Education.ExploreEducationStatistics.Common.Services; using GovUk.Education.ExploreEducationStatistics.Common.Services.Interfaces; using GovUk.Education.ExploreEducationStatistics.Common.Tests.Extensions; using GovUk.Education.ExploreEducationStatistics.Common.Tests.Fixtures; using GovUk.Education.ExploreEducationStatistics.Content.Model.Database; using GovUk.Education.ExploreEducationStatistics.Data.Model.Database; -using Microsoft.AspNetCore.Mvc.Testing; -using Microsoft.Extensions.Configuration; -using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; -using Microsoft.Extensions.Logging; using Moq; -using Testcontainers.Azurite; namespace GovUk.Education.ExploreEducationStatistics.Content.Api.Tests.Fixtures; public sealed class TestApplicationFactory : TestApplicationFactory { - private readonly AzuriteContainer _azuriteContainer = new AzuriteBuilder() - .WithImage("mcr.microsoft.com/azure-storage/azurite:3.31.0") - .WithInMemoryPersistence() - .Build(); - - public override async ValueTask DisposeAsync() - { - await _azuriteContainer.DisposeAsync(); - } - - /// - /// Start the Azurite container. Once started, the test app must also - /// be configured with to use it. - /// - /// - /// We don't start the Azurite container in a class fixture as there currently - /// isn't a good way to clear it after each test. The current approach is to - /// restart the container for each test case (which is quite slow). - /// See: https://github.com/Azure/Azurite/issues/588. - /// For now, we should manually control the Azurite container's lifecycle by - /// calling this on a case-by-case basis. - /// - public async Task StartAzurite() - { - await _azuriteContainer.StartAsync(); - } - - public async Task StopAzurite() - { - await _azuriteContainer.StopAsync(); - } - - public WebApplicationFactory WithAzurite(bool enabled = true) + public async Task ClearAllTestData() { - if (!enabled) - { - return this; - } - - if (_azuriteContainer.State != TestcontainersStates.Running) - { - throw new InvalidOperationException( - $"Azurite container must be started via '{nameof(StartAzurite)}' method first"); - } - - return WithWebHostBuilder(builder => - { - builder - .ConfigureAppConfiguration((_, config) => - { - config.AddInMemoryCollection( - [ - new KeyValuePair("PublicStorage", _azuriteContainer.GetConnectionString()) - ]); - }) - .ConfigureServices(services => - { - services.ReplaceService(sp => - new PublicBlobStorageService( - _azuriteContainer.GetConnectionString(), - sp.GetRequiredService>() - ) - ); - }); - }); + await EnsureDatabaseDeleted(); + await EnsureDatabaseDeleted(); } protected override IHostBuilder CreateHostBuilder() diff --git a/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/MethodologyGeneratorExtensions.cs b/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/MethodologyGeneratorExtensions.cs index 12c6dc1bc79..4ee528301a0 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/MethodologyGeneratorExtensions.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/MethodologyGeneratorExtensions.cs @@ -58,16 +58,24 @@ private static InstanceSetters SetMethodologyVersions( this InstanceSetters setters, Func> methodologyVersions) => setters.Set( - m => m.Versions, - (_, methodology, context) => - { - var list = methodologyVersions.Invoke(context).ToList(); + m => m.Versions, + (_, methodology, context) => + { + var list = methodologyVersions.Invoke(context).ToList(); - list.ForEach(methodologyVersion => methodologyVersion.Methodology = methodology); + list.ForEach(methodologyVersion => methodologyVersion.Methodology = methodology); - return list; - } - ); + var latestPublishedVersion = list + .Where(mv => mv.Published.HasValue) + .OrderBy(mv => mv.Published!) + .LastOrDefault(); + + methodology.LatestPublishedVersion = latestPublishedVersion; + methodology.LatestPublishedVersionId = latestPublishedVersion?.Id; + + return list; + } + ); private static InstanceSetters SetLatestPublishedVersion( this InstanceSetters setters, diff --git a/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/MethodologyVersionGeneratorExtensions.cs b/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/MethodologyVersionGeneratorExtensions.cs index 4c909e32735..6ebfe049ac6 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/MethodologyVersionGeneratorExtensions.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/MethodologyVersionGeneratorExtensions.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Linq; using GovUk.Education.ExploreEducationStatistics.Common.Extensions; using GovUk.Education.ExploreEducationStatistics.Common.Tests.Fixtures; @@ -38,6 +39,16 @@ public static Generator WithPublished( DateTime published) => generator.ForInstance(d => d.SetPublished(published)); + public static Generator WithRedirects( + this Generator generator, + IEnumerable methodologyRedirects) + => generator.ForInstance(s => s.SetRedirects(methodologyRedirects)); + + public static Generator WithRedirects( + this Generator generator, + Func> methodologyRedirects) + => generator.ForInstance(s => s.SetRedirects(methodologyRedirects.Invoke)); + public static InstanceSetters SetDefaults(this InstanceSetters setters) => setters .SetDefault(p => p.Id) @@ -60,4 +71,27 @@ public static InstanceSetters SetPublished( this InstanceSetters setters, DateTime published) => setters.Set(mv => mv.Published, published); + + public static InstanceSetters SetRedirects( + this InstanceSetters setters, + IEnumerable methodologyRedirects) + => setters.SetRedirects(_ => methodologyRedirects); + + private static InstanceSetters SetRedirects( + this InstanceSetters setters, + Func> methodologyRedirects) + => setters.Set( + mv => mv.MethodologyRedirects, + (_, methodologyVersion, context) => + { + var list = methodologyRedirects.Invoke(context).ToList(); + + list.ForEach(methodologyRedirect => + { + methodologyRedirect.MethodologyVersion = methodologyVersion; + methodologyRedirect.MethodologyVersionId = methodologyVersion.Id; + }); + + return list; + }); } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/PublicationGeneratorExtensions.cs b/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/PublicationGeneratorExtensions.cs index 989976bb1c4..9a22c3c570c 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/PublicationGeneratorExtensions.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/PublicationGeneratorExtensions.cs @@ -80,6 +80,11 @@ public static Generator WithTheme( Theme theme) => generator.ForInstance(s => s.SetTheme(theme)); + public static Generator WithSlug( + this Generator generator, + string slug) + => generator.ForInstance(s => s.SetSlug(slug)); + public static InstanceSetters SetId( this InstanceSetters setters, Guid id) @@ -106,6 +111,16 @@ public static Generator WithThemes( return generator; } + public static Generator WithRedirects( + this Generator generator, + IEnumerable publicationRedirects) + => generator.ForInstance(s => s.SetRedirects(publicationRedirects)); + + public static Generator WithRedirects( + this Generator generator, + Func> publicationRedirects) + => generator.ForInstance(s => s.SetRedirects(publicationRedirects.Invoke)); + public static InstanceSetters SetReleases( this InstanceSetters setters, IEnumerable releases) @@ -270,4 +285,32 @@ private static InstanceSetters SetTheme( Theme theme) => setters.Set(p => p.Theme, theme) .SetThemeId(theme.Id); + + public static InstanceSetters SetSlug( + this InstanceSetters setters, + string slug) + => setters.Set(p => p.Slug, slug); + + public static InstanceSetters SetRedirects( + this InstanceSetters setters, + IEnumerable publicationRedirects) + => setters.SetRedirects(_ => publicationRedirects); + + private static InstanceSetters SetRedirects( + this InstanceSetters setters, + Func> publicationRedirects) + => setters.Set( + mv => mv.PublicationRedirects, + (_, publication, context) => + { + var list = publicationRedirects.Invoke(context).ToList(); + + list.ForEach(publicationRedirect => + { + publicationRedirect.Publication = publication; + publicationRedirect.PublicationId = publication.Id; + }); + + return list; + }); } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/ReleaseGeneratorExtensions.cs b/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/ReleaseGeneratorExtensions.cs index 5ebea8621a9..2ed14ecaa44 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/ReleaseGeneratorExtensions.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Model.Tests/Fixtures/ReleaseGeneratorExtensions.cs @@ -97,6 +97,21 @@ public static Generator WithYear( int year) => generator.ForInstance(s => s.SetYear(year)); + public static Generator WithSlug( + this Generator generator, + string slug) + => generator.ForInstance(s => s.SetSlug(slug)); + + public static Generator WithRedirects( + this Generator generator, + IEnumerable releaseRedirects) + => generator.ForInstance(s => s.SetRedirects(releaseRedirects)); + + public static Generator WithRedirects( + this Generator generator, + Func> releaseRedirects) + => generator.ForInstance(s => s.SetRedirects(releaseRedirects.Invoke)); + public static Generator WithLabel( this Generator generator, string? label) @@ -240,6 +255,34 @@ public static InstanceSetters SetYear( int year) => setters.Set(p => p.Year, year); + public static InstanceSetters SetSlug( + this InstanceSetters setters, + string slug) + => setters.Set(p => p.Slug, slug); + + public static InstanceSetters SetRedirects( + this InstanceSetters setters, + IEnumerable releaseRedirects) + => setters.SetRedirects(_ => releaseRedirects); + + private static InstanceSetters SetRedirects( + this InstanceSetters setters, + Func> releaseRedirects) + => setters.Set( + mv => mv.ReleaseRedirects, + (_, release, context) => + { + var list = releaseRedirects.Invoke(context).ToList(); + + list.ForEach(releaseRedirect => + { + releaseRedirect.Release = release; + releaseRedirect.ReleaseId = release.Id; + }); + + return list; + }); + public static InstanceSetters SetLabel( this InstanceSetters setters, string? label) diff --git a/src/GovUk.Education.ExploreEducationStatistics.Content.Model/MethodologyVersion.cs b/src/GovUk.Education.ExploreEducationStatistics.Content.Model/MethodologyVersion.cs index 1a2dc950440..9fc2ed2083f 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.Model/MethodologyVersion.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Model/MethodologyVersion.cs @@ -42,6 +42,8 @@ public class MethodologyVersion : ICreatedTimestamp public List Notes { get; set; } = new(); + public List MethodologyRedirects { get; set; } = []; + public Methodology Methodology { get; set; } = null!; public Guid MethodologyId { get; set; } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Content.Model/Publication.cs b/src/GovUk.Education.ExploreEducationStatistics.Content.Model/Publication.cs index 8386148e2c3..5f15ace12eb 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.Model/Publication.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Model/Publication.cs @@ -21,6 +21,8 @@ public class Publication public List ReleaseVersions { get; set; } = []; + public List PublicationRedirects { get; set; } = []; + public List Methodologies { get; set; } = []; public ExternalMethodology? ExternalMethodology { get; set; } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Content.Model/PublicationRedirect.cs b/src/GovUk.Education.ExploreEducationStatistics.Content.Model/PublicationRedirect.cs index 08881ca4d7a..14da8256319 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.Model/PublicationRedirect.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Model/PublicationRedirect.cs @@ -1,4 +1,4 @@ -#nullable enable +#nullable enable using System; using GovUk.Education.ExploreEducationStatistics.Common.Model; @@ -6,11 +6,11 @@ namespace GovUk.Education.ExploreEducationStatistics.Content.Model; public class PublicationRedirect : ICreatedTimestamp { - public string Slug { get; init; } = null!; + public string Slug { get; set; } = null!; - public Guid PublicationId { get; init; } + public Guid PublicationId { get; set; } - public Publication Publication { get; init; } = null!; + public Publication Publication { get; set; } = null!; public DateTime Created { get; set; } } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Content.Model/Release.cs b/src/GovUk.Education.ExploreEducationStatistics.Content.Model/Release.cs index 699e6998cb5..aa7bc75d42d 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.Model/Release.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Model/Release.cs @@ -28,6 +28,8 @@ public class Release : ICreatedUpdatedTimestamps public List Versions { get; set; } = []; + public List ReleaseRedirects { get; set; } = []; + public DateTime Created { get; set; } public DateTime? Updated { get; set; } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Content.Model/ReleaseRedirect.cs b/src/GovUk.Education.ExploreEducationStatistics.Content.Model/ReleaseRedirect.cs index 0b6d93f226f..5efe0ab2466 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.Model/ReleaseRedirect.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Model/ReleaseRedirect.cs @@ -6,11 +6,11 @@ namespace GovUk.Education.ExploreEducationStatistics.Content.Model; public class ReleaseRedirect : ICreatedTimestamp { - public string Slug { get; init; } = null!; + public string Slug { get; set; } = null!; - public Guid ReleaseId { get; init; } + public Guid ReleaseId { get; set; } - public Release Release { get; init; } = null!; + public Release Release { get; set; } = null!; public DateTime Created { get; set; } } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Content.Services.Tests/RedirectsServiceTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Content.Services.Tests/RedirectsServiceTests.cs index 19ab2c5c116..33c729d978f 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.Services.Tests/RedirectsServiceTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Services.Tests/RedirectsServiceTests.cs @@ -11,136 +11,6 @@ namespace GovUk.Education.ExploreEducationStatistics.Content.Services.Tests { public class RedirectsServiceTests { - [Fact] - public async Task List_PublicationRedirect() - { - var publication = new Publication - { - Slug = "redirect-to", - }; - - var publicationRedirects = new List - { - new() - { - Slug = "redirect-from-2", - Publication = publication, - }, - new() - { - Slug = "redirect-from-1", - Publication = publication, - }, - new() - { - Slug = "redirect-to", // should be excluded in results, as same as current publication slug - Publication = publication, - }, - }; - - var contentDbContextId = Guid.NewGuid().ToString(); - await using (var contentDbContext = InMemoryContentDbContext(contentDbContextId)) - { - await contentDbContext.PublicationRedirects.AddRangeAsync(publicationRedirects); - await contentDbContext.SaveChangesAsync(); - } - - await using (var contentDbContext = InMemoryContentDbContext(contentDbContextId)) - { - var redirectsService = SetupRedirectsService(contentDbContext); - - var result = await redirectsService.List(); - - var viewModel = result.AssertRight(); - var publicationsRedirectViewModel = viewModel.Publications; - - Assert.Equal(2, publicationsRedirectViewModel.Count); - - Assert.Equal("redirect-from-2", publicationsRedirectViewModel[0].FromSlug); - Assert.Equal("redirect-to", publicationsRedirectViewModel[0].ToSlug); - - Assert.Equal("redirect-from-1", publicationsRedirectViewModel[1].FromSlug); - Assert.Equal("redirect-to", publicationsRedirectViewModel[1].ToSlug); - } - } - - [Fact] - public async Task List_MethodologyRedirect() - { - var latestPublishedVersionId = Guid.NewGuid(); - var methodology = new Methodology - { - LatestPublishedVersionId = latestPublishedVersionId, - OwningPublicationSlug = "no-redirect-to-1", - Versions = new List - { - new() - { - // previous version - AlternativeSlug = "no-redirect-to-2", - Version = 0, - }, - new() - { - Id = latestPublishedVersionId, - AlternativeSlug = "redirect-to", - Version = 1, - }, - new() - { - // latestVersion but unpublished - AlternativeSlug = "no-redirect-to-3", - Version = 2, - }, - } - }; - - var methodologyRedirects = new List - { - new() - { - Slug = "redirect-from-2", - MethodologyVersion = methodology.Versions[0], - }, - new() - { - Slug = "redirect-from-1", - MethodologyVersion = methodology.Versions[1], - }, - new() - { - Slug = "no-redirect-from-1", - MethodologyVersion = methodology.Versions[2], - }, - }; - - var contentDbContextId = Guid.NewGuid().ToString(); - await using (var contentDbContext = InMemoryContentDbContext(contentDbContextId)) - { - await contentDbContext.Methodologies.AddAsync(methodology); - await contentDbContext.MethodologyRedirects.AddRangeAsync(methodologyRedirects); - await contentDbContext.SaveChangesAsync(); - } - - await using (var contentDbContext = InMemoryContentDbContext(contentDbContextId)) - { - var redirectsService = SetupRedirectsService(contentDbContext); - - var result = await redirectsService.List(); - - var viewModel = result.AssertRight(); - var methodologyRedirectViewModel = viewModel.Methodologies; - - Assert.Equal(2, methodologyRedirectViewModel.Count); - - Assert.Equal("redirect-from-2", methodologyRedirectViewModel[0].FromSlug); - Assert.Equal("redirect-to", methodologyRedirectViewModel[0].ToSlug); - - Assert.Equal("redirect-from-1", methodologyRedirectViewModel[1].FromSlug); - Assert.Equal("redirect-to", methodologyRedirectViewModel[1].ToSlug); - } - } - [Fact] public async Task List_MethodologyRedirect_MethodologyNotPublished() { @@ -178,7 +48,7 @@ public async Task List_MethodologyRedirect_MethodologyNotPublished() var viewModel = result.AssertRight(); - Assert.Empty(viewModel.Methodologies); + Assert.Empty(viewModel.MethodologyRedirects); } } @@ -231,9 +101,9 @@ public async Task List_MethodologyRedirect_PreviousVersionRedirectOnly() var result = await redirectsService.List(); var viewModel = result.AssertRight(); - var methodologyRedirectViewModel = viewModel.Methodologies; + var methodologyRedirectsViewModel = viewModel.MethodologyRedirects; - var redirect = Assert.Single(methodologyRedirectViewModel); + var redirect = Assert.Single(methodologyRedirectsViewModel); Assert.Equal("redirect-from-1", redirect.FromSlug); Assert.Equal("redirect-to", redirect.ToSlug); @@ -318,7 +188,7 @@ public async Task List_MethodologyRedirect_MultipleMethodologies() var result = await redirectsService.List(); var viewModel = result.AssertRight(); - var methodologyRedirectsViewModel = viewModel.Methodologies; + var methodologyRedirectsViewModel = viewModel.MethodologyRedirects; Assert.Equal(2, methodologyRedirectsViewModel.Count); @@ -379,7 +249,7 @@ public async Task List_MethodologyRedirect_FilterRedirectIfSameAsCurrentSlug() var result = await redirectsService.List(); var viewModel = result.AssertRight(); - Assert.Empty(viewModel.Methodologies); + Assert.Empty(viewModel.MethodologyRedirects); } } @@ -438,7 +308,7 @@ public async Task List_MethodologyRedirect_DuplicateRedirects() var viewModel = result.AssertRight(); - var redirect = Assert.Single(viewModel.Methodologies); + var redirect = Assert.Single(viewModel.MethodologyRedirects); Assert.Equal("duplicated-redirect", redirect.FromSlug); Assert.Equal("redirect-to", redirect.ToSlug); } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Content.Services/RedirectsService.cs b/src/GovUk.Education.ExploreEducationStatistics.Content.Services/RedirectsService.cs index 10a392a8be3..6f03b39aa12 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.Services/RedirectsService.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Services/RedirectsService.cs @@ -22,7 +22,7 @@ public RedirectsService(ContentDbContext contentDbContext) public async Task> List() { - var publicationRedirectViewModels = await _contentDbContext.PublicationRedirects + var publicationRedirects = await _contentDbContext.PublicationRedirects .Where(pr => pr.Slug != pr.Publication.Slug) // don't use redirects to the current live slug .Distinct() .Select(pr => new RedirectViewModel( @@ -34,21 +34,21 @@ public async Task> List() // A redirect for a MethodologyVersion that isn't published shouldn't appear in the list. A redirect becomes // active once the associated MethodologyVersion is published. It remains active if subsequent methodology // amendments are published. We establish this by checking against each MethodologyVersion's Version number. - var methodologyRedirects = await _contentDbContext.MethodologyRedirects - .Where(mr => mr.MethodologyVersion.Methodology.LatestPublishedVersion != null - && mr.MethodologyVersion.Methodology.LatestPublishedVersion.Version >= - mr.MethodologyVersion.Version + var methodologyRedirects = ( + await _contentDbContext.MethodologyRedirects + .Where(mr => mr.MethodologyVersion.Methodology.LatestPublishedVersion != null + && mr.MethodologyVersion.Methodology.LatestPublishedVersion.Version >= + mr.MethodologyVersion.Version + ) + .Select(mr => new + { + RedirectSlug = mr.Slug, + // MethodologyVersion.Slug cannot be translated into SQL, so we do this... + LatestPublishedSlug = mr.MethodologyVersion.Methodology.LatestPublishedVersion!.AlternativeSlug + ?? mr.MethodologyVersion.Methodology.OwningPublicationSlug, + }) + .ToListAsync() ) - .Select(mr => new - { - RedirectSlug = mr.Slug, - // MethodologyVersion.Slug cannot be translated into SQL, so we do this... - LatestPublishedSlug = mr.MethodologyVersion.Methodology.LatestPublishedVersion!.AlternativeSlug - ?? mr.MethodologyVersion.Methodology.OwningPublicationSlug, - }) - .ToListAsync(); - - var methodologyRedirectViewModels = methodologyRedirects .Select(mr => { if (mr.RedirectSlug == mr.LatestPublishedSlug) @@ -64,18 +64,28 @@ public async Task> List() .Distinct() .ToList(); - var releaseRedirectViewModels = await _contentDbContext.ReleaseRedirects - .Where(rr => rr.Slug != rr.Release.Slug) // don't use redirects to the current live slug - .Distinct() - .Select(rr => new RedirectViewModel( - rr.Slug, - rr.Release.Slug - )) - .ToListAsync(); + var releaseRedirectsByPublicationSlug = ( + await _contentDbContext.ReleaseRedirects + .Where(rr => rr.Slug != rr.Release.Slug) // don't use redirects to the current live slug + .Distinct() + .Select(rr => new + { + PublicationSlug = rr.Release.Publication.Slug, + FromSlug = rr.Slug, + ToSlug = rr.Release.Slug + }) + .ToListAsync() + ) + .GroupBy(rr => rr.PublicationSlug) + .ToDictionary( + g => g.Key, + g => g + .Select(a => new RedirectViewModel(FromSlug: a.FromSlug, ToSlug: a.ToSlug)) + .ToList()); return new RedirectsViewModel( - publicationRedirectViewModels, - methodologyRedirectViewModels, - releaseRedirectViewModels); + PublicationRedirects: publicationRedirects, + MethodologyRedirects: methodologyRedirects, + ReleaseRedirectsByPublicationSlug: releaseRedirectsByPublicationSlug); } } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Content.ViewModels/RedirectViewModels.cs b/src/GovUk.Education.ExploreEducationStatistics.Content.ViewModels/RedirectViewModels.cs index e211a620400..5c68b68e2d2 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.ViewModels/RedirectViewModels.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.ViewModels/RedirectViewModels.cs @@ -1,9 +1,9 @@ namespace GovUk.Education.ExploreEducationStatistics.Content.ViewModels; public record RedirectsViewModel( - List Publications, - List Methodologies, - List Releases); + List PublicationRedirects, + List MethodologyRedirects, + Dictionary> ReleaseRedirectsByPublicationSlug); public record RedirectViewModel( string FromSlug, diff --git a/src/explore-education-statistics-frontend/src/middleware/pages/__tests__/redirectPages.test.ts b/src/explore-education-statistics-frontend/src/middleware/pages/__tests__/redirectPages.test.ts index 328a848ce0b..772cc444b81 100644 --- a/src/explore-education-statistics-frontend/src/middleware/pages/__tests__/redirectPages.test.ts +++ b/src/explore-education-statistics-frontend/src/middleware/pages/__tests__/redirectPages.test.ts @@ -15,11 +15,11 @@ describe('redirectPages', () => { const nextSpy = jest.spyOn(NextResponse, 'next'); const testRedirects: Redirects = { - methodologies: [ + methodologyRedirects: [ { fromSlug: 'original-slug-1', toSlug: 'updated-slug-1' }, { fromSlug: 'original-slug-2', toSlug: 'updated-slug-2' }, ], - publications: [ + publicationRedirects: [ { fromSlug: 'original-slug-3', toSlug: 'updated-slug-3' }, { fromSlug: 'original-slug-4', toSlug: 'updated-slug-4' }, ], diff --git a/src/explore-education-statistics-frontend/src/middleware/pages/redirectPages.ts b/src/explore-education-statistics-frontend/src/middleware/pages/redirectPages.ts index e9c4e07bbbd..ea5e6a09509 100644 --- a/src/explore-education-statistics-frontend/src/middleware/pages/redirectPages.ts +++ b/src/explore-education-statistics-frontend/src/middleware/pages/redirectPages.ts @@ -15,8 +15,8 @@ const cacheTime = getCacheTime(); let cachedRedirects: CachedRedirects | undefined; const redirectPaths = { - methodologies: '/methodology', - publications: '/find-statistics', + methodologyRedirects: '/methodology', + publicationRedirects: '/find-statistics', }; export default async function redirectPages( diff --git a/src/explore-education-statistics-frontend/src/services/redirectService.ts b/src/explore-education-statistics-frontend/src/services/redirectService.ts index 4daf2724edb..f90347cc773 100644 --- a/src/explore-education-statistics-frontend/src/services/redirectService.ts +++ b/src/explore-education-statistics-frontend/src/services/redirectService.ts @@ -1,6 +1,6 @@ export interface Redirects { - methodologies: Redirect[]; - publications: Redirect[]; + methodologyRedirects: Redirect[]; + publicationRedirects: Redirect[]; } export type RedirectType = keyof Redirects;