Skip to content

EES-5934 Fixing Admin Public API Data Set Changelog page #5705

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,79 @@ private async Task<HttpResponseMessage> ListLiveVersions(
}
}

public class GetDataSetVersionTests(
TestApplicationFactory testApp) : DataSetVersionsControllerTests(testApp)
{
public static TheoryData<DataSetVersionStatus> AllDataSetVersionStatuses => new(EnumUtil.GetEnums<DataSetVersionStatus>());

[Theory]
[MemberData(nameof(AllDataSetVersionStatuses))]
public async Task Success(DataSetVersionStatus dataSetVersionStatus)
{
DataSet dataSet = DataFixture
.DefaultDataSet()
.WithStatusPublished();

await TestApp.AddTestData<PublicDataDbContext>(context => context.DataSets.Add(dataSet));

var dataSetVersions = DataFixture
.DefaultDataSetVersion()
.WithStatus(dataSetVersionStatus)
.WithDataSet(dataSet)
.GenerateList(3);

await TestApp.AddTestData<PublicDataDbContext>(context =>
{
context.DataSetVersions.AddRange(dataSetVersions);
context.DataSets.Update(dataSet);
});

var requestedDataSetVersion = dataSetVersions[1];

var response = await GetDataSetVersion(dataSetVersionId: requestedDataSetVersion.Id);

var viewModel = response.AssertOk<DataSetVersionInfoViewModel>();

Assert.NotNull(viewModel);
Assert.Equal(requestedDataSetVersion.Id, viewModel.Id);
Assert.Equal(requestedDataSetVersion.PublicVersion, viewModel.Version);
Assert.Equal(requestedDataSetVersion.Status, viewModel.Status);
Assert.Equal(requestedDataSetVersion.VersionType, viewModel.Type);
Assert.Equal(requestedDataSetVersion.Notes, viewModel.Notes);
}

[Fact]
public async Task NotBauUser_Returns403()
{
var client = BuildApp(user: DataFixture.AuthenticatedUser()).CreateClient();

var response = await GetDataSetVersion(
dataSetVersionId: Guid.NewGuid(),
client: client);

response.AssertForbidden();
}

[Fact]
public async Task DataSetVersionDoesNotExist_Returns404()
{
var response = await GetDataSetVersion(dataSetVersionId: Guid.NewGuid());

response.AssertNotFound();
}

private async Task<HttpResponseMessage> GetDataSetVersion(
Guid dataSetVersionId,
HttpClient? client = null)
{
client ??= BuildApp().CreateClient();

var uri = $"{BaseUrl}/{dataSetVersionId}";

return await client.GetAsync(uri);
}
}

public class CreateNextVersionTests(
TestApplicationFactory testApp) : DataSetVersionsControllerTests(testApp)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,19 @@ public async Task<ActionResult<PaginatedListViewModel<DataSetLiveVersionSummaryV
.HandleFailuresOrOk();
}

[HttpGet("{dataSetVersionId:guid}")]
[Produces("application/json")]
public async Task<ActionResult<DataSetVersionInfoViewModel>> GetDataSetVersion(
Guid dataSetVersionId,
CancellationToken cancellationToken)
{
return await dataSetVersionService
.GetDataSetVersion(
dataSetVersionId: dataSetVersionId,
cancellationToken: cancellationToken)
.HandleFailuresOrOk();
}

[HttpPost]
[Produces("application/json")]
public async Task<ActionResult<DataSetVersionSummaryViewModel>> CreateNextVersion(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@
using GovUk.Education.ExploreEducationStatistics.Admin.Services.Methodologies;
using GovUk.Education.ExploreEducationStatistics.Admin.ViewModels;
using GovUk.Education.ExploreEducationStatistics.Admin.ViewModels.ManageContent;
using GovUk.Education.ExploreEducationStatistics.Admin.ViewModels.Public.Data;
using GovUk.Education.ExploreEducationStatistics.Common.Extensions;
using GovUk.Education.ExploreEducationStatistics.Common.Mappings;
using GovUk.Education.ExploreEducationStatistics.Common.ViewModels;
using GovUk.Education.ExploreEducationStatistics.Content.Model;
using GovUk.Education.ExploreEducationStatistics.Content.ViewModels;
using GovUk.Education.ExploreEducationStatistics.Public.Data.Model;
using GovUk.Education.ExploreEducationStatistics.Publisher.Model;
using System;
using System.Collections.Generic;
Expand Down Expand Up @@ -211,6 +213,14 @@ public MappingProfiles()
methodologyVersion.Notes.OrderByDescending(note => note.DisplayDate)));

CreateMap<ReleaseVersion, ReleasePublicationStatusViewModel>();

CreateMap<DataSetVersion, DataSetVersionInfoViewModel>()
.ForMember(dest => dest.Version,
m => m.MapFrom(dataSetVersion =>
dataSetVersion.PublicVersion))
.ForMember(dest => dest.Type,
m => m.MapFrom(dataSetVersion =>
dataSetVersion.VersionType));
}

private void CreateContentBlockMap()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,19 @@ Task<Either<ActionResult, PaginatedListViewModel<DataSetLiveVersionSummaryViewMo
int pageSize,
CancellationToken cancellationToken = default);

Task<List<DataSetVersionStatusSummary>> GetStatusesForReleaseVersion(
Guid releaseVersionId,
Task<Either<ActionResult, DataSetVersionInfoViewModel>> GetDataSetVersion(
Guid dataSetVersionId,
CancellationToken cancellationToken = default);

Task<Either<ActionResult, DataSetVersion>> GetDataSetVersion(
Guid dataSetId,
SemVersion version,
CancellationToken cancellationToken = default);

Task<List<DataSetVersionStatusSummary>> GetStatusesForReleaseVersion(
Guid releaseVersionId,
CancellationToken cancellationToken = default);

Task<Either<ActionResult, Unit>> DeleteVersion(
Guid dataSetVersionId,
CancellationToken cancellationToken = default);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Net.Http;
using System.Threading;
using System.Threading.Tasks;
using AutoMapper;
using GovUk.Education.ExploreEducationStatistics.Admin.Requests.Public.Data;
using GovUk.Education.ExploreEducationStatistics.Admin.Services.Interfaces.Public.Data;
using GovUk.Education.ExploreEducationStatistics.Admin.Services.Interfaces.Security;
Expand Down Expand Up @@ -32,7 +33,8 @@ public class DataSetVersionService(
PublicDataDbContext publicDataDbContext,
IProcessorClient processorClient,
IPublicDataApiClient publicDataApiClient,
IUserService userService)
IUserService userService,
IMapper mapper)
: IDataSetVersionService
{
public async Task<Either<ActionResult, PaginatedListViewModel<DataSetLiveVersionSummaryViewModel>>>
Expand Down Expand Up @@ -73,6 +75,31 @@ public async Task<Either<ActionResult, PaginatedListViewModel<DataSetLiveVersion
});
}

public async Task<Either<ActionResult, DataSetVersionInfoViewModel>> GetDataSetVersion(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't want to call this GetDataSetVersionInfo ?

Guid dataSetVersionId,
CancellationToken cancellationToken = default)
{
return await userService.CheckIsBauUser()
.OnSuccess(async () => await GetVersion(
dataSetVersionId: dataSetVersionId,
cancellationToken: cancellationToken))
.OnSuccess(mapper.Map<DataSetVersionInfoViewModel>);
}

public async Task<Either<ActionResult, DataSetVersion>> GetDataSetVersion(
Guid dataSetId,
SemVersion version,
CancellationToken cancellationToken = default)
{
return await publicDataDbContext.DataSetVersions
.AsNoTracking()
.Include(dsv => dsv.DataSet)
.Where(dsv => dsv.DataSetId == dataSetId)
.Where(dsv => dsv.VersionMajor == version.Major)
.Where(dsv => dsv.VersionMinor == version.Minor)
.SingleOrNotFoundAsync(cancellationToken);
}

public async Task<List<DataSetVersionStatusSummary>> GetStatusesForReleaseVersion(
Guid releaseVersionId,
CancellationToken cancellationToken = default)
Expand Down Expand Up @@ -129,20 +156,6 @@ public async Task<Either<ActionResult, DataSetVersionSummaryViewModel>> Complete
.OnSuccess(async dataSetVersion => await MapDraftVersionSummary(dataSetVersion, cancellationToken));
}

public async Task<Either<ActionResult, DataSetVersion>> GetDataSetVersion(
Guid dataSetId,
SemVersion version,
CancellationToken cancellationToken = default)
{
return await publicDataDbContext.DataSetVersions
.AsNoTracking()
.Include(dsv => dsv.DataSet)
.Where(dsv => dsv.DataSetId == dataSetId)
.Where(dsv => dsv.VersionMajor == version.Major)
.Where(dsv => dsv.VersionMinor == version.Minor)
.SingleOrNotFoundAsync(cancellationToken);
}

public async Task<Either<ActionResult, Unit>> DeleteVersion(
Guid dataSetVersionId,
CancellationToken cancellationToken = default)
Expand Down Expand Up @@ -175,7 +188,7 @@ public async Task<Either<ActionResult, DataSetDraftVersionViewModel>> UpdateVers
CancellationToken cancellationToken = default)
{
return await userService.CheckIsBauUser()
.OnSuccess(async () => await GetDataSetVersion(
.OnSuccess(async () => await GetVersion(
dataSetVersionId: dataSetVersionId,
cancellationToken: cancellationToken))
.OnSuccessDo(dataSetVersion => CheckCanUpdateVersion(dataSetVersion, updateRequest))
Expand Down Expand Up @@ -267,7 +280,7 @@ private async Task<DataSetVersionSummaryViewModel> MapDraftVersionSummary(
};
}

private async Task<Either<ActionResult, DataSetVersion>> GetDataSetVersion(
private async Task<Either<ActionResult, DataSetVersion>> GetVersion(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this naming makes sense as it isn't returning a version.

Guid dataSetVersionId,
CancellationToken cancellationToken)
{
Expand Down
13 changes: 10 additions & 3 deletions src/GovUk.Education.ExploreEducationStatistics.Admin/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -872,11 +872,11 @@ public Task<Either<ActionResult, PaginatedListViewModel<DataSetLiveVersionSummar
return Task.FromResult(PaginatedListViewModel<DataSetLiveVersionSummaryViewModel>.Paginate([], 1, 10));
}

public Task<List<DataSetVersionStatusSummary>> GetStatusesForReleaseVersion(
Guid releaseVersionId,
public Task<Either<ActionResult, DataSetVersionInfoViewModel>> GetDataSetVersion(
Guid dataSetVersionIdId,
CancellationToken cancellationToken = default)
{
return Task.FromResult(new List<DataSetVersionStatusSummary>());
return Task.FromResult(new Either<ActionResult, DataSetVersionInfoViewModel>(new NotFoundResult()));
}

public Task<Either<ActionResult, DataSetVersion>> GetDataSetVersion(
Expand All @@ -887,6 +887,13 @@ public Task<Either<ActionResult, DataSetVersion>> GetDataSetVersion(
return Task.FromResult(new Either<ActionResult, DataSetVersion>(new NotFoundResult()));
}

public Task<List<DataSetVersionStatusSummary>> GetStatusesForReleaseVersion(
Guid releaseVersionId,
CancellationToken cancellationToken = default)
{
return Task.FromResult(new List<DataSetVersionStatusSummary>());
}

public Task<Either<ActionResult, DataSetVersionSummaryViewModel>> CreateNextVersion(
Guid releaseFileId,
Guid dataSetId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,3 +85,18 @@ public record MappingStatusViewModel

public required bool FiltersComplete { get; init; }
}

public record DataSetVersionInfoViewModel
{
public required Guid Id { get; init; }

public required string Version { get; init; }

[JsonConverter(typeof(StringEnumConverter))]
public required DataSetVersionStatus Status { get; init; }

[JsonConverter(typeof(StringEnumConverter))]
public required DataSetVersionType Type { get; init; }

public required string Notes { get; init; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ import { useQuery } from '@tanstack/react-query';
import { generatePath, useParams } from 'react-router-dom';
import React, { useEffect } from 'react';
import WarningMessage from '@common/components/WarningMessage';
import { DataSetVersionStatus } from '@admin/services/apiDataSetService';

const dataSetVersionIsDraft = (dataSetVersionStatus: DataSetVersionStatus) =>
dataSetVersionStatus === 'Draft';
Comment on lines +25 to +26
Copy link
Collaborator

@bennettstuart bennettstuart Mar 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: seems a bit verbose, where we set isDraft this is fine
const isDraft = dataSetVersion?.status === 'Draft'


export default function ReleaseApiDataSetChangelogPage() {
const { dataSetId, dataSetVersionId, releaseVersionId, publicationId } =
Expand All @@ -31,18 +35,20 @@ export default function ReleaseApiDataSetChangelogPage() {
refetch: refetchDataSet,
} = useQuery(apiDataSetQueries.get(dataSetId));

const { data: dataSetVersion, isLoading: isLoadingDataSetVersion } = useQuery(
apiDataSetVersionQueries.getVersion(dataSetVersionId),
);

const { data: changes, isLoading: isLoadingChanges } = useQuery(
apiDataSetVersionQueries.getChanges(dataSetVersionId),
);

const isDraft = dataSet?.draftVersion?.id === dataSetVersionId;
const isDraft = dataSetVersion
? dataSetVersionIsDraft(dataSetVersion.status)
: false;

const [showForm, toggleShowForm] = useToggle(false);

const dataSetVersion = isDraft
? dataSet?.draftVersion
: dataSet?.latestLiveVersion;

useEffect(() => {
if (isDraft && !dataSetVersion?.notes) {
toggleShowForm.on();
Expand Down Expand Up @@ -76,7 +82,11 @@ export default function ReleaseApiDataSetChangelogPage() {
Back to API data set details
</Link>

<LoadingSpinner loading={isLoadingDataSet || isLoadingChanges}>
<LoadingSpinner
loading={
isLoadingDataSet || isLoadingDataSetVersion || isLoadingChanges
}
>
{dataSet && dataSetVersion ? (
<>
<div className="govuk-grid-row">
Expand Down
Loading