Skip to content

Commit

Permalink
Merge pull request #5444 from dfe-analytical-services/EES-5656_Releas…
Browse files Browse the repository at this point in the history 
…e_Series_Tidyup

EES-5656 Release series tidy up
  • Loading branch information
@benoutram
benoutram authored Dec 10, 2024
2 parents b72ed29 + 504166c commit 2116dcf
Show file tree
Hide file tree
Showing 21 changed files with 296 additions and 431 deletions.
7 changes: 6 additions & 1 deletion ...tion.ExploreEducationStatistics.Admin.Tests/Services/PublicationServicePermissionTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,12 @@ await PermissionTestUtils.PolicyCheckBuilder<SecurityPolicies>()
var service = BuildPublicationService(
context: contentDbContext,
userService: userService.Object);
return await service.AddReleaseSeriesLegacyLink(publication.Id, new ReleaseSeriesLegacyLinkAddRequest());
return await service.AddReleaseSeriesLegacyLink(publication.Id,
new ReleaseSeriesLegacyLinkAddRequest
{
Description = "Test description",
Url = "https://test.url"
});
});
}

Expand Down
468 changes: 183 additions & 285 deletions ...ovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/PublicationServiceTests.cs
View file

Large diffs are not rendered by default.

18 changes: 8 additions & 10 deletions src/GovUk.Education.ExploreEducationStatistics.Admin/Requests/ReleaseSeriesItemRequests.cs
View file
Original file line number Diff line number Diff line change
@@ -1,21 +1,19 @@
#nullable enable

using System;

namespace GovUk.Education.ExploreEducationStatistics.Admin.Requests;

public class ReleaseSeriesLegacyLinkAddRequest
public record ReleaseSeriesLegacyLinkAddRequest
{
public string Description { get; set; } = string.Empty;
public string Url { get; set; } = string.Empty;
public required string Description { get; init; }

public required string Url { get; init; }
}

public class ReleaseSeriesItemUpdateRequest
public record ReleaseSeriesItemUpdateRequest
{
public Guid Id { get; set; }
public Guid? ReleaseId { get; set; }
public Guid? ReleaseId { get; init; }

public string? LegacyLinkDescription { get; set; }
public string? LegacyLinkUrl { get; set; }
public string? LegacyLinkDescription { get; init; }
public string? LegacyLinkUrl { get; init; }
}

5 changes: 0 additions & 5 deletions ...ation.ExploreEducationStatistics.Admin/Services/ManageContent/ManageContentPageService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,6 @@ await _releaseVersionRepository
{
return new ReleaseSeriesItemViewModel
{
Id = rsi.Id,
IsLegacyLink = rsi.IsLegacyLink,
Description = rsi.LegacyLinkDescription!,
LegacyLinkUrl = rsi.LegacyLinkUrl,
};
Expand All @@ -130,10 +128,7 @@ await _releaseVersionRepository

return new ReleaseSeriesItemViewModel
{
Id = rsi.Id,
IsLegacyLink = rsi.IsLegacyLink,
Description = latestReleaseVersion.Title,

ReleaseId = latestReleaseVersion.ReleaseId,
ReleaseSlug = latestReleaseVersion.Slug,
};
Expand Down
85 changes: 29 additions & 56 deletions src/GovUk.Education.ExploreEducationStatistics.Admin/Services/PublicationService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,12 @@
using Microsoft.EntityFrameworkCore;
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Threading.Tasks;
using GovUk.Education.ExploreEducationStatistics.Admin.Services.Interfaces;
using static GovUk.Education.ExploreEducationStatistics.Admin.Validators.ValidationErrorMessages;
using static GovUk.Education.ExploreEducationStatistics.Admin.Validators.ValidationUtils;
using ExternalMethodologyViewModel = GovUk.Education.ExploreEducationStatistics.Admin.ViewModels.ExternalMethodologyViewModel;
using IPublicationRepository = GovUk.Education.ExploreEducationStatistics.Admin.Services.Interfaces.IPublicationRepository;
using IPublicationService = GovUk.Education.ExploreEducationStatistics.Admin.Services.Interfaces.IPublicationService;
using IReleaseVersionRepository = GovUk.Education.ExploreEducationStatistics.Content.Model.Repository.Interfaces.IReleaseVersionRepository;
using PublicationViewModel = GovUk.Education.ExploreEducationStatistics.Admin.ViewModels.PublicationViewModel;
using ReleaseSummaryViewModel = GovUk.Education.ExploreEducationStatistics.Admin.ViewModels.ReleaseSummaryViewModel;
Expand Down Expand Up @@ -435,11 +433,12 @@ public async Task<Either<ActionResult, List<ReleaseSummaryViewModel>>> ListLates
});
}

public async Task<Either<ActionResult, List<ReleaseSeriesTableEntryViewModel>>> GetReleaseSeries(Guid publicationId)
public async Task<Either<ActionResult, List<ReleaseSeriesTableEntryViewModel>>> GetReleaseSeries(
Guid publicationId)
{
return await _persistenceHelper
.CheckEntityExists<Publication>(publicationId)
.OnSuccess(publication => _userService.CheckCanViewPublication(publication))
return await _context.Publications
.FirstOrNotFoundAsync(p => p.Id == publicationId)
.OnSuccess(_userService.CheckCanViewPublication)
.OnSuccess(async publication =>
{
var result = new List<ReleaseSeriesTableEntryViewModel>();
Expand All @@ -450,43 +449,28 @@ public async Task<Either<ActionResult, List<ReleaseSeriesTableEntryViewModel>>>
result.Add(new ReleaseSeriesTableEntryViewModel
{
Id = seriesItem.Id,
IsLegacyLink = true,
Description = seriesItem.LegacyLinkDescription!,
LegacyLinkUrl = seriesItem.LegacyLinkUrl,
});
}
else
{
// prefer getting the latest published version over an unpublished amendment
var latestVersion = await _context.ReleaseVersions
.LatestReleaseVersion(seriesItem.ReleaseId!.Value, publishedOnly: true)
.SingleOrDefaultAsync();
var release = await _context.Releases
.SingleAsync(r => r.Id == seriesItem.ReleaseId);

if (latestVersion == null)
{
// if the release has no published version, then use its original unpublished version
latestVersion = await _context.ReleaseVersions
.LatestReleaseVersion(seriesItem.ReleaseId!.Value)
.SingleOrDefaultAsync();

if (latestVersion == null)
{
throw new InvalidDataException(
"ReleaseSeriesItem with ReleaseId set should have an associated " +
$"ReleaseVersion. Release: {seriesItem.ReleaseId} " +
$"ReleaseSeriesItem: {seriesItem.Id}");
}
}
var latestPublishedReleaseVersion = await _context.ReleaseVersions
.LatestReleaseVersion(releaseId: seriesItem.ReleaseId!.Value, publishedOnly: true)
.SingleOrDefaultAsync();

result.Add(new ReleaseSeriesTableEntryViewModel
{
Id = seriesItem.Id,
IsLegacyLink = false,
Description = latestVersion.Title,
ReleaseId = latestVersion.ReleaseId,
ReleaseSlug = latestVersion.Slug,
IsLatest = latestVersion.Id == publication.LatestPublishedReleaseVersionId,
IsPublished = latestVersion.Live,
ReleaseId = release.Id,
Description = release.Title,
ReleaseSlug = release.Slug,
IsLatest = publication.LatestPublishedReleaseVersionId != null &&
latestPublishedReleaseVersion?.Id == publication.LatestPublishedReleaseVersionId,
IsPublished = latestPublishedReleaseVersion != null
});
}
}
Expand All @@ -507,7 +491,6 @@ public async Task<Either<ActionResult, List<ReleaseSeriesTableEntryViewModel>>>
publication.ReleaseSeries.Add(new ReleaseSeriesItem
{
Id = Guid.NewGuid(),
ReleaseId = null,
LegacyLinkDescription = newLegacyLink.Description,
LegacyLinkUrl = newLegacyLink.Url,
});
Expand All @@ -517,8 +500,7 @@ public async Task<Either<ActionResult, List<ReleaseSeriesTableEntryViewModel>>>

await _publicationCacheService.UpdatePublication(publication.Slug);

return await GetReleaseSeries(publication.Id)
.OnSuccess(releaseSeries => releaseSeries);
return await GetReleaseSeries(publication.Id);
});
}

Expand All @@ -527,7 +509,6 @@ public async Task<Either<ActionResult, List<ReleaseSeriesTableEntryViewModel>>>
List<ReleaseSeriesItemUpdateRequest> updatedReleaseSeriesItems)
{
return await _context.Publications
.Include(p => p.ReleaseVersions)
.FirstOrNotFoundAsync(p => p.Id == publicationId)
.OnSuccess(_userService.CheckCanManageReleaseSeries)
.OnSuccess(async publication =>
Expand All @@ -538,56 +519,48 @@ public async Task<Either<ActionResult, List<ReleaseSeriesTableEntryViewModel>>>
if (seriesItem.ReleaseId != null && (
seriesItem.LegacyLinkDescription != null || seriesItem.LegacyLinkUrl != null))
{
throw new ArgumentException(
$"LegacyLink details shouldn't be set if ReleaseId is set. ReleaseSeriesItem: {seriesItem.Id}");
throw new ArgumentException("LegacyLink details shouldn't be set if ReleaseId is set.");
}

if (seriesItem.ReleaseId == null && (
seriesItem.LegacyLinkDescription == null || seriesItem.LegacyLinkUrl == null))
{
throw new ArgumentException(
$"LegacyLink details should be set if ReleaseId is null. ReleaseSeriesItem: {seriesItem.Id}");
throw new ArgumentException("LegacyLink details should be set if ReleaseId is null.");
}
}

// Check all publication releases are included in updatedReleaseSeriesItems
var publicationReleaseIds = publication.ReleaseVersions
.Select(rv => rv.ReleaseId)
.Distinct()
.ToList();
var publicationReleaseIds = await _context.Releases
.Where(r => r.PublicationId == publicationId)
.Select(r => r.Id)
.ToListAsync();

var updatedSeriesReleaseIds = updatedReleaseSeriesItems
.Where(rsi => rsi.ReleaseId != null)
.Where(rsi => rsi.ReleaseId.HasValue)
.Select(rsi => rsi.ReleaseId!.Value)
.ToList();

if (!ComparerUtils.SequencesAreEqualIgnoringOrder(publicationReleaseIds, updatedSeriesReleaseIds))
{
throw new ArgumentException(
"Missing or duplicate release in new release series. Expected ReleaseIds: " +
publicationReleaseIds.Select(id => id.ToString()).JoinToString(","));
publicationReleaseIds.JoinToString(","));
}

// NOTE: A malicious user could change the release series items' Ids, but we don't care

var newReleaseSeries = updatedReleaseSeriesItems
publication.ReleaseSeries = updatedReleaseSeriesItems
.Select(request => new ReleaseSeriesItem
{
Id = request.Id,
Id = Guid.NewGuid(),
ReleaseId = request.ReleaseId,
LegacyLinkDescription = request.LegacyLinkDescription,
LegacyLinkUrl = request.LegacyLinkUrl,
}).ToList();

publication.ReleaseSeries = newReleaseSeries;
_context.Publications.Update(publication);

await _context.SaveChangesAsync();

await _publicationCacheService.UpdatePublication(publication.Slug);

return await GetReleaseSeries(publication.Id)
.OnSuccess(releaseSeries => releaseSeries);
return await GetReleaseSeries(publication.Id);
});
}

Expand Down
2 changes: 1 addition & 1 deletion src/GovUk.Education.ExploreEducationStatistics.Admin/Services/ReleaseService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ await CreateGenericContentFromTemplate(releaseCreate.TemplateReleaseId.Value,
publication.ReleaseSeries.Insert(0, new ReleaseSeriesItem
{
Id = Guid.NewGuid(),
ReleaseId = newReleaseVersion.ReleaseId,
ReleaseId = release.Id
});
_context.Publications.Update(publication);

Expand Down
18 changes: 9 additions & 9 deletions ...Education.ExploreEducationStatistics.Admin/ViewModels/ReleaseSeriesTableEntryViewModel.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,17 @@ namespace GovUk.Education.ExploreEducationStatistics.Admin.ViewModels;

public record ReleaseSeriesTableEntryViewModel
{
public Guid Id { get; set; }
public bool IsLegacyLink { get; set; }
public string Description { get; set; } = string.Empty;
public required Guid Id { get; init; }
public required string Description { get; init; }

// used by EES release series item
public Guid? ReleaseId { get; set; }
public string? ReleaseSlug { get; set; }
public bool? IsLatest { get; set; }
public bool? IsPublished { get; set; }
public Guid? ReleaseId { get; init; }
public string? ReleaseSlug { get; init; }
public bool? IsLatest { get; init; }
public bool? IsPublished { get; init; }

// used by legacy link series item
public string? LegacyLinkUrl { get; set; }
}
public string? LegacyLinkUrl { get; init; }

public bool IsLegacyLink => ReleaseId == null;
}
12 changes: 6 additions & 6 deletions ...ovUk.Education.ExploreEducationStatistics.Common/ViewModels/ReleaseSeriesItemViewModel.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ namespace GovUk.Education.ExploreEducationStatistics.Common.ViewModels;

public record ReleaseSeriesItemViewModel
{
public Guid Id { get; set; }
public bool IsLegacyLink { get; set; }
public string Description { get; set; } = string.Empty;
public required string Description { get; init; }

// used by EES release series item
public Guid? ReleaseId { get; set; }
public string? ReleaseSlug { get; set; }
public Guid? ReleaseId { get; init; }
public string? ReleaseSlug { get; init; }

// used by legacy link series item
public string? LegacyLinkUrl { get; set; }
public string? LegacyLinkUrl { get; init; }

public bool IsLegacyLink => ReleaseId == null;
}
3 changes: 3 additions & 0 deletions src/GovUk.Education.ExploreEducationStatistics.Content.Model/ReleaseSeriesItem.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ namespace GovUk.Education.ExploreEducationStatistics.Content.Model;

public record ReleaseSeriesItem
{
/// <summary>
/// Unique identifier for the ReleaseSeriesItem which exists to allow safely managing legacy links in the UI.
/// </summary>
public Guid Id { get; set; }

public Guid? ReleaseId { get; set; }
Expand Down
10 changes: 4 additions & 6 deletions ...n.ExploreEducationStatistics.Content.Services.Tests/Cache/PublicationCacheServiceTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,14 @@ public class PublicationCacheServiceTests : CacheServiceTestFixture
Title = ""
}
},
ReleaseSeries = new()
{
new()
ReleaseSeries =
[
new ReleaseSeriesItemViewModel
{
Id = Guid.NewGuid(),
IsLegacyLink = true,
Description = "legacy link description",
LegacyLinkUrl = "http://test.com/",
}
},
],
Theme = new ThemeViewModel(
Guid.NewGuid(),
Slug: "",
Expand Down
1 change: 0 additions & 1 deletion ...Uk.Education.ExploreEducationStatistics.Content.Services.Tests/PublicationServiceTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,6 @@ public async Task Success()
Assert.Null(releaseSeriesItem2.LegacyLinkUrl);

var releaseSeriesItem3 = publicationViewModel.ReleaseSeries[2];
Assert.Equal(_legacyLinks[0].Id, releaseSeriesItem3.Id);
Assert.True(releaseSeriesItem3.IsLegacyLink);
Assert.Null(releaseSeriesItem3.ReleaseId);
Assert.Equal(_legacyLinks[0].LegacyLinkDescription, releaseSeriesItem3.Description);
Expand Down
5 changes: 0 additions & 5 deletions src/GovUk.Education.ExploreEducationStatistics.Content.Services/PublicationService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,6 @@ public async Task<Either<ActionResult, PublicationCacheViewModel>> Get(string pu
{
return new ReleaseSeriesItemViewModel
{
Id = rsi.Id,
IsLegacyLink = rsi.IsLegacyLink,
Description = rsi.LegacyLinkDescription!,
LegacyLinkUrl = rsi.LegacyLinkUrl,
};
Expand All @@ -112,10 +110,7 @@ public async Task<Either<ActionResult, PublicationCacheViewModel>> Get(string pu

return new ReleaseSeriesItemViewModel
{
Id = rsi.Id,
IsLegacyLink = rsi.IsLegacyLink,
Description = latestReleaseVersion.Title,

ReleaseId = latestReleaseVersion.ReleaseId,
ReleaseSlug = latestReleaseVersion.Slug,
};
Expand Down
1 change: 0 additions & 1 deletion ...ion-statistics-admin/src/pages/publication/PublicationEditReleaseSeriesLegacyLinkPage.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ export const mapToReleaseSeriesItemUpdateRequest = (
releaseSeries: ReleaseSeriesTableEntry[],
): ReleaseSeriesItemUpdateRequest[] => {
return releaseSeries.map(seriesItem => ({
id: seriesItem.id,
releaseId: seriesItem.releaseId,
legacyLinkDescription: seriesItem.isLegacyLink
? seriesItem.description
Expand Down
Loading

0 comments on commit 2116dcf

Please sign in to comment.