Skip to content
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

Updates and support for re-use of CMS logic in Deploy #14990

Conversation

AndyButland
Copy link
Contributor

Prerequisites

  • I have added steps to test this contribution in the description below

Description

This PR contains two updates that we'd like to make for Umbraco 13 in order to simplify code in Deploy.

Amend to IFileSource interface

I've added an extra parameter to the interface method IFileSource.GetFilesAsync. We have a use for this where we want to continue if a file matching one of the provided UDIs isn't found in some cases, and in others, throw a FileNotFoundException. Hence the need for the parameter. Currently we are working around this by casting to the concrete type, but it would be cleaner if it were on the interface.

There are no uses of this interface within the CMS itself.

Migrate logic for redirect tracking and creation to a service

Currently this logic is within a notification handler - RedirectTrackingHandler - which means the logic within can't easily be re-used in Deploy where we also need to track content moves and name changes in order to create redirects. As such we have this logic effectively duplicated in existing versions of Deploy.

In this PR I've moved the logic into a service - IRedirectTracker / RedirectTracker - from where we should be able to re-use it in Deploy rather than duplicating.

I've also included some refactoring and minor bug fixing work that @ronaldbarendse carried out when reviewing and updating this for Deploy.

To Test:

  • Create some redirect records by updating content in such a way that it's URL will change. This can be done by moving it to a different location in the tree, or changing the name such that the URL segment for the content item, and hence the full path, will update.
  • Check that redirects are created and visible under Info > Redirect URL Management for the content item.

@AndyButland AndyButland changed the title Updates and supporting re-use of CMS logic in Deploy Updates and support for re-use of CMS logic in Deploy Oct 17, 2023
Copy link
Member

@bergmania bergmania left a comment

Choose a reason for hiding this comment

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

A small change request, but the rest looks good to me

///// <param name="Udi">A file entity identifier.</param>
///// <returns>A byte array containing the file content.</returns>
// byte[] GetFileBytes(StringUdi Udi);
Task GetFilesAsync(IEnumerable<StringUdi> udis, IFileTypeCollection fileTypes, bool continueOnFileNotFound, CancellationToken token);
Copy link
Member

Choose a reason for hiding this comment

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

IMO we should have the old signature with a default implementation that makes this part not breaking

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have applied that amend.

@bergmania bergmania merged commit d08d141 into v13/dev Oct 31, 2023
5 of 8 checks passed
@bergmania bergmania deleted the v13/feature/introduce-redirect-handler-and-add-parameter-to-ifilesource branch October 31, 2023 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants