EES-5639 - Adding the ability to be able to edit the `Label` for any Release (BE) #5591

jack-hive · 2025-02-05T16:09:02Z

Purpose of PR

We want the ability for certain (admin) users to be able to edit the Label for any Release. We want them to have the ability to be able to do this at any stage (for both draft AND published Releases).

We want to be able to do this at the Publications level - i.e. on the Publication's 'Releases' page; not just on the 'Release Summary' page. Hence, we have introduced a new endpoint in his PR for editing the Label of a Release. Currently, the endpoint only allows you to edit the Label, but more parameters may be added to the request model in the future.

This PR encompasses the BE work. The FE work to add a new interface to the UI for a user to edit the Label will be undertaken in EES-5640.

The users we are allowing to use this new endpoint, and we have added authorization handlers for, must have one of the following:

Be a BAU user
Have the UpdateAllPublications Claim
Have the Owner Publication Role

Considerations made for the new endpoint

When editing the Label, we need to update the Release's Slug.
When editing the Label, we need to validate that no other Release has the same combination of ReleaseName/TimePeriodCoverage/Label. This combination, which is used to construct the Slug, must be unique - otherwise, we could potentially have multiple Releases which are for the same time period and iteration ('iteration' being the Label).
When editing the Label, we need to create a new Release Redirect, pointing from the old Slug to the new Slug.
When editing the Label, we need to invalidate and update any cache entries which may still refer to the old Release Slug
We want to block the user from editing the Label when the Release is in a state of Approved but NOT published. This is to prevent issues with the cache in the case where someone managed to edit the Label between the time when the content is moved to the staging directory (in Blob storage), and the time it is then moved from the staging directory to the public directory. [This is due to the consequence of the Slug (and hence cache) also changing when the Label changes].

Other/Related Changes

When making these changes, it was apparent that we should ideally add the new endpoint to a new Controller specific to the Release resource. Previously, we had an UpdateRelease Controller method in the ReleasesController, but this was actually used by the 'Release Summary' page in the UI to update parameters relating to the ReleaseVersion resource. In the spirit of trying to make the REST API better suit the domain model, we have done the following:
- Created a NEW ReleasesController for the Release resource specific endpoints - this contains the NEW UpdateRelease endpoint (used to update the Label)
- Renamed the OLD ReleasesController to ReleaseVersionsController for the ReleaseVersion resource specific endpoints, and renamed the UpdateRelease endpoint to UpdateReleaseVersion
- Moved the CreateRelease endpoint from the ReleaseVersionsController to the new ReleasesController
Renamed corresponding instances referring to Release when they really mean ReleaseVersion. Examples include, but aren't limited to:
- ReleaseViewModel --> ReleaseVersionViewModel
- ReleaseUpdateRequest --> ReleaseVersionUpdateRequest
- ReleasesControllerIntegrationTests --> ReleaseVersionsControllerIntegrationTests
- releaseService.ts --> releaseVersionService.ts on the FE
- ReleaseService.cs --> ReleaseVersionService.cs on the BE
- CanUpdateSpecificRelease --> CanUpdateSpecificReleaseVersion Security Policy
- ReleaseRequestValidators.cs --> ReleaseVersionRequestValidators.cs

Main changes (to focus the reviewer's attention)

Below is a list of the main files which have new changes specific to the new endpoint. Most other changes are due to renames. Hopefully this is helpful!

C# files

ReleaseVersionsController.cs
ReleasesController.cs
ReleaseVersionService.cs
ReleaseService.cs
IReleaseVersionService.cs
IReleaseService.cs
ReleaseRequests.cs
ReleaseVersionRequests.cs
UpdateSpecificReleaseVersionAuthorizationHandler.cs
UpdateSpecificReleaseAuthorizationHandler.cs
SecurityPolicies.cs
StartupSecurityConfiguration.cs
Startup.cs
UserServiceExtensions.cs
ReleaseVersionRequestValidators.cs
ReleaseRequestValidators.cs
ValidationErrorMessages.cs
ReleaseViewModels.cs
IPrivateBlobCacheService.cs
IPublicBlobCacheService.cs
PrivateBlobCacheService.cs
PublicBlobCacheService.cs
ReleaseCacheService.cs
IReleaseCacheService.cs

C# test files

ReleaseCacheServiceTests.cs
ReleaseVersionsControllerTests.cs
ReleasesControllerTests.cs
ReleaseVersionServiceTests.cs
ReleaseServiceTests.cs
IntegrationTestFixture.cs
GovUk.Education.ExploreEducationStatistics.Admin.Tests.csproj
UpdateSpecificReleaseAuthorizationHandlerTests.cs
UpdateSpecificReleaseVersionAuthorizationHandlerTests.cs
AuthorizationHandlersTestUtil.cs
PublicationAuthorizationHandlersTestUtil.cs
ReleaseServicePermissionTests.cs
ReleaseVersionServicePermissionTests.cs

FE TypeScript files

releaseVersionService.ts
releaseService.ts

FE UI test files

admin_api.py

UI test summary

I have 7 failing UI tests locally. Namely:

Admin.Bau.Create Data Block With Chart - Check updated footnote is displayed in release content page
- $${\color{green}Known \space Failure}$$
Admin.Bau.Release Status - Navigate to data upload and confirm data replacement
- $${\color{green}Known \space Failure}$$
General Public.Miscellaneous - Validate Feedback page
- $${\color{red}Only \space Failing \space Locally}$$ --> $${\color{green}Now \space PASSED, \space see \space updates \ below}$$
General Public.Table Tool Absence By Characteristic - Validate back takes you to step 1
- $${\color{orange}Known \space Flakey}$$
Public Api.Public Api Cancel And Removal - Search for API data set
- $${\color{orange}Known \space Flakey}$$
Public Api.Public Api Minor Manual Changes - User verifies minor changes in the 'API data set changelog' section
- $${\color{green}Known \space Failure}$$
Public Api.Public Api Preview Token - Search for API data set
- $${\color{red}Only \space Failing \space Locally}$$ --> $${\color{green}Now \space PASSED, \space see \space updates \ below}$$

UPDATE

Test suites
Public Api.Public Api Preview Token - Search for API data set
and
General Public.Miscellaneous - Validate Feedback page
all $${\color{green}PASSED}$$ after a re-run

benoutram · 2025-02-12T11:26:06Z

tests/robot-tests/tests/libs/admin_api.py

-        f"/api/publications/{publication_id}/releases",
+        f"/api/releases",


This comment is completely unrelated to the UI tests and this change but there's an additional project https://github.com/dfe-analytical-services/explore-education-statistics/tree/9cf05689bdc14c97f82091b88608d2666ad54df3/useful-scripts/misc-test-utils which is a CLI tool to perform useful actions and one of those allows you to create a release.

I asked on Slack and nobody is using the tool and we aren't actively maintaining it. However it looks like it might be a one line change to patch it up, similar to the change you are making here. See

explore-education-statistics/useful-scripts/misc-test-utils/src/services/releaseService.ts

Line 19 in 9cf0568

`/api/publications/${publicationId}/releases`,

.

If you give it a try and find there's too many other things already broken which stop you from testing it then I would say it's not worth the extra time to fix and test it. @N-moh said she will create a ticket checking what's useful to keep and to see if the service layer can reuse more of our existing frontend code rather than reimplementing it.

Ah, I didn't realise this existed!

I've now fixed it. I tested it and it seems to work :)

...Statistics.Admin/Security/AuthorizationHandlers/DeleteSpecificCommentAuthorizationHandler.cs

src/GovUk.Education.ExploreEducationStatistics.Admin/Services/ThemeService.cs

src/GovUk.Education.ExploreEducationStatistics.Common/Services/PublicBlobCacheService.cs

src/GovUk.Education.ExploreEducationStatistics.Admin/Services/ReleaseService.cs

benoutram · 2025-02-12T16:37:39Z

src/GovUk.Education.ExploreEducationStatistics.Admin/Services/ReleaseService.cs

+        // Update release-specific path cache
+        await releaseCacheService.UpdateRelease(
+            releaseVersionId: latestReleaseVersionId,
+            publicationSlug: publicationSlug,
+            releaseSlug: newReleaseSlug);


Since the cached release is getting removed in the previous few lines, the UpdateRelease is going to recreate it in the cache under the new slug. This isn't absolutely necessary because it should get recreated the next time the release content page is requested anyway. The important bit was removing it.

I think you could remove this if you feel like it simplifies things and replace it with a comment to explain that the latest-release.json is getting updated but there's no need to update the specific release cache.

Or feel free to keep it in!

Hmmmmm I'm torn on that one! My only argument for keeping it in, is that it's likely that once the edit modal/page is closed, this data is almost guaranteed to be reloaded. So it might make sense just to do it here, to make it clear what is going on. And It's probably ever so slightly more efficient too?

benoutram · 2025-02-12T16:51:22Z

src/GovUk.Education.ExploreEducationStatistics.Admin/Services/ReleaseService.cs


-            // TODO EES-2747 - this should be looked at to see how best to reuse similar "set to draft" logic in MethodologyApprovalService.
-            methodologiesScheduledWithRelease.ForEach(m =>
+    private async Task<Either<ActionResult, Unit>> ValidateReleaseIsNotUndergoingPublishing(Guid releaseId, CancellationToken cancellationToken)


This method is checking if the release has any version which is approved but not published, which would block an update if a version was scheduled to be published at any time in the future, until that time passes and the release is published even though publishing is not necessarily in progress right now. That could potentially be days away.

I think you should check if this behaviour is going to be ok?

If you want to check whether publishing has really started and the release has a latest version which is unpublished, you can use the version id with ReleasePublishingStatusRepository.GetAllByOverallStage and stage Started. If there's any result then you know publishing is in progress.

There's an example of that being used in

explore-education-statistics/src/GovUk.Education.ExploreEducationStatistics.Admin/Security/AuthorizationHandlers/ReleaseStatusAuthorizationHandlers.cs

Lines 39 to 43 in a7c66f0

var statuses = await _releasePublishingStatusRepository

.GetAllByOverallStage(

releaseVersion.Id,

ReleasePublishingStatusOverallStage.Started,

ReleasePublishingStatusOverallStage.Complete);

which checks to make sure publishing hasn't started or already completed before allowing any approval status change.

Never thought of that! I think you're right, I need to determine if the release is actually undergoing publishing, not just if it's queued up for it.

So I believe I need to use the logic you have mentioned there.

I'll need someone to re-review this most likely, to see if the change I have made looks right.

…oid confusion with the upcoming new endpoint

…anging the PUT endpoint for `ReleaseVersions` The `PUT` route for updating a `ReleaseVersion` has been changed to better reflect the resource it is changing: It was: `PUT /releases/{releaseVersionId}` We've changed it to: `PATCH /releaseVersions/{releaseVersionId}` It only represents a partial update; hence the switch to using `PATCH` rather than `PUT`. This has been done in preparation for a new Update endpoint for the `Release` itself in a future commit. If we did not change the route for this endpoint now, we'd have conflicting routes.

…specific public methods

…UpdateSpecificReleaseVersion`

…stValidators`, and moving the `ReleaseCreateRequestValidator` into a new `ReleaseRequestValidators`

…ests` to a new `ReleaseServiceTests`

…ePermissionTests` into `ReleaseServicePermissionTests`

…e fact that they're actually checking roles based on the `ReleaseVersion`, not a `Release`

… `UpdateSpecificReleaseVersionAuthorizationHandlerTests`

…thorization handler + tests

…el`]

…ase label

…ervice` interface

…elease` `Label`/`Slug`

… the tests

jack-hive force-pushed the EES-5639 branch 3 times, most recently from 75383aa to 032e02e Compare February 10, 2025 16:08

benoutram self-requested a review February 11, 2025 09:46

benoutram self-assigned this Feb 11, 2025

benoutram reviewed Feb 12, 2025

View reviewed changes

jack-hive force-pushed the EES-5639 branch from 6abc222 to 4b2fb76 Compare February 18, 2025 16:25

jack-hive added 23 commits February 19, 2025 10:43

EES-5639 Renaming Release => ReleaseVersion in a few places to av…

375925f

…oid confusion with the upcoming new endpoint

EES-5639 Renaming ReleasesController --> ReleaseVersionsController

8789199

EES-5639 Renaming FE releaseService --> releaseVersionService

491296d

EES-5639 Moving the createRelease API proxy to a new releaseService

d6037a9

EES-5639 Renaming ReleaseService --> ReleaseVersionService

6e2cf24

EES-5639 BE - Updating Controller tests

3a58f19

EES-5639 Creating a new ReleaseService for the ReleaseController-…

fb1f5f3

…specific public methods

EES-5639 Renaming ReleaseRequests --> ReleaseVersionRequests

b21d05f

EES-5639 Moving ReleaseCreateRequest to the ReleaseRequests

92dff43

EES-5639 Renaming CanUpdateSpecificRelease Security Policy --> `Can…

6ac283f

…UpdateSpecificReleaseVersion`

EES-5639 Renaming ReleaseRequestValidators --> `ReleaseVersionReque…

4ea61fd

…stValidators`, and moving the `ReleaseCreateRequestValidator` into a new `ReleaseRequestValidators`

EES-5639 Moving the CreateReleaseTests from `ReleaseVersionServiceT…

0be72b4

…ests` to a new `ReleaseServiceTests`

EES-5639 Fixing ReleasesControllerTests

6d0b95b

EES-5639 Moving the CreateRelease test out of `ReleaseVersionServic…

fdde651

…ePermissionTests` into `ReleaseServicePermissionTests`

EES-5639 Fixing unit tests

e42b015

EES-5639 Renaming AuthorizationHandlerService methods to reflect th…

e0963d3

…e fact that they're actually checking roles based on the `ReleaseVersion`, not a `Release`

EES-5639 Rename UpdateSpecificReleaseAuthorizationHandlerTests -- >…

6cb3db2

… `UpdateSpecificReleaseVersionAuthorizationHandlerTests`

EES-5639 Adding new CanUpdateSpecificRelease Security Policy and au…

7ab5f38

…thorization handler + tests

EES-5639 Adding new UpdateRelease endpoint [Used to update the `Lab…

df221df

…el`]

EES-5639 Fixing issues introduced from rebase

f7d2d0e

EES-5639 Clearing cache and creating redirects when changing the rele…

849185c

…ase label

EES-5639 [WIP] Adding integration tests

3a705be

jack-hive added 11 commits February 19, 2025 10:44

EES-5639 Adding a new IPrivateBlobCacheService & `IPublicBlobCacheS…

a954365

…ervice` interface

EES-5639 Fixing existing integration tests

620a549

EES-5639 Clearing the Publication cache view-model on updating the `R…

14ce7b3

…elease` `Label`/`Slug`

EES-5639 Simplifying some logic when deleting Release blob cache

8a6fccc

EES-5639 Fixing dependency injection bug

e7eab68

EES-5639 Fixing robot tests Release creation API

7f23b5b

EES-5639 Fixing TypeScript compiler errors

67fc332

EES-5639 Fixing prettier format checks

758c521

EES-5639 Changes as per PR reviee

baa93be

EES-5639 Fixing CLI tool command used to create new Releases

94a2419

EES-5639 Updating caching tests - Renames + an extra check for one of…

e3ab129

… the tests

jack-hive force-pushed the EES-5639 branch from 96ee69a to e3ab129 Compare February 19, 2025 10:44

EES-5639 Removing unused imports

75bc0ff

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EES-5639 - Adding the ability to be able to edit the `Label` for any Release (BE) #5591

EES-5639 - Adding the ability to be able to edit the `Label` for any Release (BE) #5591

jack-hive commented Feb 5, 2025 •

edited

Loading

benoutram Feb 12, 2025

jack-hive Feb 18, 2025

benoutram Feb 12, 2025

jack-hive Feb 13, 2025 •

edited

Loading

benoutram Feb 12, 2025 •

edited

Loading

jack-hive Feb 13, 2025

jack-hive Feb 13, 2025

		f"/api/publications/{publication_id}/releases",
		f"/api/releases",

	var statuses = await _releasePublishingStatusRepository
	.GetAllByOverallStage(
	releaseVersion.Id,
	ReleasePublishingStatusOverallStage.Started,
	ReleasePublishingStatusOverallStage.Complete);

EES-5639 - Adding the ability to be able to edit the Label for any Release (BE) #5591

Are you sure you want to change the base?

EES-5639 - Adding the ability to be able to edit the Label for any Release (BE) #5591

Conversation

jack-hive commented Feb 5, 2025 • edited Loading

Purpose of PR

Considerations made for the new endpoint

Other/Related Changes

Main changes (to focus the reviewer's attention)

C# files

C# test files

FE TypeScript files

FE UI test files

UI test summary

UPDATE

benoutram Feb 12, 2025

Choose a reason for hiding this comment

jack-hive Feb 18, 2025

Choose a reason for hiding this comment

benoutram Feb 12, 2025

Choose a reason for hiding this comment

jack-hive Feb 13, 2025 • edited Loading

Choose a reason for hiding this comment

benoutram Feb 12, 2025 • edited Loading

Choose a reason for hiding this comment

jack-hive Feb 13, 2025

Choose a reason for hiding this comment

jack-hive Feb 13, 2025

Choose a reason for hiding this comment

EES-5639 - Adding the ability to be able to edit the `Label` for any Release (BE) #5591

EES-5639 - Adding the ability to be able to edit the `Label` for any Release (BE) #5591

jack-hive commented Feb 5, 2025 •

edited

Loading

jack-hive Feb 13, 2025 •

edited

Loading

benoutram Feb 12, 2025 •

edited

Loading