EES-5405 Stash all release related fields of a data set version in a new Release entity #5157
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
benoutram
force-pushed
the
EES-5405
branch
3 times, most recently
from
d0ada44 to
88f5a6f
Compare
jack-hive
reviewed
Aug 19, 2024
...vUk.Education.ExploreEducationStatistics.Admin/Services/Public.Data/DataSetVersionService.cs
Show resolved
Hide resolved
...vUk.Education.ExploreEducationStatistics.Admin/Services/Public.Data/DataSetVersionService.cs
Show resolved
Hide resolved
jack-hive
requested changes
Aug 19, 2024
src/GovUk.Education.ExploreEducationStatistics.Admin/Services/Public.Data/DataSetService.cs
Show resolved
Hide resolved
src/GovUk.Education.ExploreEducationStatistics.Admin/Services/Public.Data/DataSetService.cs
Show resolved
Hide resolved
src/GovUk.Education.ExploreEducationStatistics.Admin/Services/Public.Data/DataSetService.cs
Show resolved
Hide resolved
| .WithRelease(DataFixture.DefaultDataSetVersionRelease() | ||
| .WithReleaseFileId(releaseVersion1DataFile.Id) | ||
| .WithSlug(releaseVersion1.Slug) | ||
| .WithTitle(releaseVersion2.Title)); |
There was a problem hiding this comment.
think this should be WithTitle(releaseVersion1.Title)
There was a problem hiding this comment.
Good spot! Fixed now, thanks. 🙂
jack-hive
approved these changes
Aug 20, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR simplifies Public API endpoints for getting a data set version and listing data set versions by removing the need to make an external call to the Content API to get release file and release information related to data set versions.
The fields currently gathered from the external Content API client call are the
File.DataSetFileId,ReleaseVersion.Slug, andReleaseVersion.Title, all from the content model.This PR makes a Public Data model change to stash all of these release related fields of a data set version in a new
Releaseowned entity owned byDataSetVersion. Existing fieldDataSetVersion.ReleaseFileIdis also migrated to the new entity.As well as removing the need for the external call, this change makes getting a data set version compatible with preview tokens in #5133. Attempting to get a data set version with a preview token currently fails due to the Content API not allowing access to unpublished data unless.
This results in one renamed column and three new columns of the
DataSetVersiondatabase table:ReleaseFileId->Release_ReleaseFileIdRelease_DataSetFileIdRelease_SlugRelease_TitleAdmin changes to update a release version
The year and time period of a draft release version can be updated in the Admin which alter the values of slug and title. This means a change has been necessary to the Admin's update release endpoint to keep any data set versions related to the release version indirectly via release files, in sync with the content release version.
Other changes
Fix a bug in
Admin.Tests.Fixture.IntegrationTestFixturewhere after clearing test data, tracked entities inChangeTrackerwere not cleared. Note, it's important to access the context in the fixture withTestApp.Services.GetRequiredService<PublicDataDbContext>()rather thanTestApp.GetDbContext<PublicDataDbContext>()to ensure we get the same Scoped lifetime context instance from the same service provider as used by DI to set up transient services, rather than from a new scope service provider which would create a new context instance where clearing the tracker would have no effect.Reformat
Admin.Tests.Services.ReleaseServiceTests, grouping related tests into separate classes.