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

Basic PR for #154 #156

Open
wants to merge 8 commits into
base: v4/main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 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 @@ -13,6 +13,10 @@ public class RedirectsContentAppSettings {
public bool Enabled { get; set; } = true;

/// <summary>
/// Gets or sets whether the user's start nodes should filter which redirects they have access to. Default is <see lanword="true"/>.
/// </summary>
public bool UserStartNodes { get; set; } = false;

Copy link
Member

Choose a reason for hiding this comment

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

Sorry - I made a mistake in my description about this property. My intention was to indicate whether to use starts nodes as the basis for permissions.

Could you rename this to UseStartsNodes?

The XML summary is also slightly wrong, as the default is false and not true. And lanword should be langword.

/// Gets or sets a list of content types and media types where the content app should or should not be shown.
/// The format follows Umbraco's <c>show</c> option - eg. <c>+content/*</c> enables the content app for all
/// content.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Net;
using Microsoft.AspNetCore.Mvc;
Expand All @@ -12,12 +13,17 @@
using Skybrud.Umbraco.Redirects.Models.Api;
using Skybrud.Umbraco.Redirects.Models.Options;
using Skybrud.Umbraco.Redirects.Services;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Models.Membership;
using Umbraco.Cms.Core.Models.PublishedContent;
using Umbraco.Cms.Core.Security;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Web;
using Umbraco.Cms.Web.BackOffice.Controllers;
using Umbraco.Cms.Web.Common.Attributes;
using Umbraco.Cms.Web.Common.UmbracoContext;
using Umbraco.Extensions;

#pragma warning disable 1591

Expand Down Expand Up @@ -61,13 +67,15 @@ public RedirectsController(ILogger<RedirectsController> logger, IRedirectsServic
[HttpGet]
public ActionResult GetRootNodes() {

RedirectRootNode[] rootNodes = _redirects.GetRootNodes();

return new JsonResult(new {
total = rootNodes.Length,
items = rootNodes.Select(x => new RedirectRootNodeModel(x, _backOffice))
});
var rootNodes = _backOffice.Settings.ContentApp.UserStartNodes
? _redirects.GetRootNodes(_backOffice.CurrentUser).ToArray()
Copy link
Member

Choose a reason for hiding this comment

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

Isn't the .ToArray() call redundant here?

Copy link
Author

Choose a reason for hiding this comment

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

My bad, caught it.

: _redirects.GetRootNodes();

return new JsonResult(new
{
total = rootNodes.Length,
items = rootNodes.Select(x => new RedirectRootNodeModel(x, _backOffice))
});
}

[HttpPost]
Expand Down Expand Up @@ -255,20 +263,23 @@ public ActionResult DeleteRedirect(Guid redirectId) {
/// <param name="limit">The maximum amount of redirects to be returned per page.</param>
/// <param name="type">The type of redirects that should be returned.</param>
/// <param name="text">The text that the returned redirects should match.</param>
/// <param name="rootNodeKey">The root node key that the returned redirects should match. <c>null</c> means all redirects. <see cref="Guid.Empty"/> means all global redirects.</param>
/// <param name="rootNodeKeys">An array of root node keys that the returned redirects should match. <c>null</c> means all redirects. <see cref="Guid.Empty"/> means all global redirects.</param>
/// <returns>A list of redirects.</returns>
[HttpGet]
public ActionResult GetRedirects(int page = 1, int limit = 20, string type = null, string text = null, Guid? rootNodeKey = null) {
public ActionResult GetRedirects(int page = 1, int limit = 20, string type = null, string text = null, [FromQuery]Guid[] rootNodeKeys = null) {

try {
var rootKeys = _backOffice.Settings.ContentApp.UserStartNodes
? _redirects.GetRootNodes(_backOffice.CurrentUser).Select(p => p.Key)
: _redirects.GetRootNodes().Select(p => p.Key);

// Initialize the search options
RedirectsSearchOptions options = new() {
Page = page,
Limit = limit,
Type = EnumUtils.ParseEnum(type, RedirectTypeFilter.All),
Text = text,
RootNodeKey = rootNodeKey
RootNodeKeys = (rootNodeKeys != null && rootNodeKeys.Any()) ? rootNodeKeys : rootKeys.ToArray()
};

// Make the search for redirects via the redirects service
Expand Down Expand Up @@ -369,7 +380,6 @@ private ActionResult Error(RedirectsException ex) {
};

}

}

}
7 changes: 7 additions & 0 deletions src/Skybrud.Umbraco.Redirects/Models/RedirectRootNode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ public class RedirectRootNode {
[JsonProperty("icon")]
public string Icon { get; }

/// <summary>
/// Gets the path of the root node.
/// </summary>
[JsonProperty("path")]
public IEnumerable<int> Path { get; set; }

/// <summary>
/// Gets the domains asscoiated with the root node.
/// </summary>
Expand All @@ -44,6 +50,7 @@ private RedirectRootNode(IContent content, IEnumerable<RedirectDomain> domains)
Id = content.Id;
Key = content.Key;
Name = content.Name;
Path = content.Path.Split(',', StringSplitOptions.RemoveEmptyEntries).Select(int.Parse);
Icon = content.ContentType.Icon;
Domains = domains?.Select(x => x.Name).ToArray() ?? Array.Empty<string>();
}
Expand Down
9 changes: 9 additions & 0 deletions src/Skybrud.Umbraco.Redirects/Services/IRedirectsService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
using Skybrud.Umbraco.Redirects.Models;
using Skybrud.Umbraco.Redirects.Models.Options;

using Umbraco.Cms.Core.Models.Membership;

namespace Skybrud.Umbraco.Redirects.Services {

/// <summary>
Expand Down Expand Up @@ -116,6 +118,13 @@ public interface IRedirectsService {
/// <returns>An array of <see cref="RedirectRootNode"/> representing the root nodes.</returns>
RedirectRootNode[] GetRootNodes();

/// <summary>
/// Returns an array of all rode nodes configured in Umbraco.
/// </summary>
/// <param name="user">An <see cref="IUser"/> object to determine access to root nodes.</param>
/// <returns>An array of <see cref="RedirectRootNode"/> representing the root nodes.</returns>
RedirectRootNode[] GetRootNodes(IUser user);

/// <summary>
/// Returns an array of redirects where the destination matches the specified <paramref name="nodeType"/> and <paramref name="nodeId"/>.
/// </summary>
Expand Down
15 changes: 11 additions & 4 deletions src/Skybrud.Umbraco.Redirects/Services/RedirectsSearchOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,19 @@ public class RedirectsSearchOptions {
/// </summary>
public string Text { get; set; }

/// <summary>
/// Gets or sets the key the returned redirects should match. <see cref="Guid.Empty"/> indicates all global
/// redirects. Default is <c>null</c>, in which case this filter is disabled.
/// </summary>
/// <summary>
/// Gets or sets the key the returned redirects should match. <see cref="Guid.Empty"/> indicates all global
/// redirects. Default is <c>null</c>, in which case this filter is disabled.
/// </summary>
[Obsolete("Obsoleted in favour of RootNodeKeys, as a user may have access to more than one root node.")]
public Guid? RootNodeKey { get; set; }

/// <summary>
/// Gets or sets the key the returned redirects should match. <see cref="Guid.Empty"/> indicates all global
/// redirects. Default is <c>null</c>, in which case this filter is disabled.
/// </summary>
public Guid[] RootNodeKeys { get; set; }

#endregion

#region Constructors
Expand Down
59 changes: 46 additions & 13 deletions src/Skybrud.Umbraco.Redirects/Services/RedirectsService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Skybrud.Umbraco.Redirects.Models;
using Skybrud.Umbraco.Redirects.Models.Dtos;
using Skybrud.Umbraco.Redirects.Models.Options;
using Umbraco.Cms.Core.Models.Membership;
using Umbraco.Cms.Core.Models.PublishedContent;
using Umbraco.Cms.Core.Routing;
using Umbraco.Cms.Core.Services;
Expand Down Expand Up @@ -366,7 +367,11 @@ public RedirectsSearchResult GetRedirects(RedirectsSearchOptions options) {
var sql = scope.SqlContext.Sql().Select<RedirectDto>().From<RedirectDto>();

// Search by the rootNodeId
if (options.RootNodeKey != null) sql = sql.Where<RedirectDto>(x => x.RootKey == options.RootNodeKey.Value);
if (options.RootNodeKeys != null && options.RootNodeKeys.Any()) {
sql = sql.Where<RedirectDto>(x => options.RootNodeKeys.Contains(x.RootKey));
} else if (options.RootNodeKey != null) {
sql = sql.Where<RedirectDto>(x => x.RootKey == options.RootNodeKey);
}

// Search by the type
if (options.Type != RedirectTypeFilter.All) {
Expand Down Expand Up @@ -395,6 +400,7 @@ public RedirectsSearchResult GetRedirects(RedirectsSearchOptions options) {
||
(x.Path.Contains(url) && x.QueryString.Contains(query))
));

}
}

Expand Down Expand Up @@ -453,19 +459,46 @@ public IEnumerable<IRedirect> GetAllRedirects() {
/// Returns an array of all rode nodes configured in Umbraco.
/// </summary>
/// <returns>An array of <see cref="RedirectRootNode"/> representing the root nodes.</returns>
public RedirectRootNode[] GetRootNodes() {

// Multiple domains may be configured for a single node, so we need to group the domains before proceeding
var domainsByRootNodeId = GetDomains().GroupBy(x => x.RootNodeId);

return (
from domainGroup in domainsByRootNodeId
let content = _contentService.GetById(domainGroup.First().RootNodeId)
where content is { Trashed: false }
orderby content.Id
select RedirectRootNode.GetFromContent(content, domainGroup)
).ToArray();
public RedirectRootNode[] GetRootNodes()
{
var domainsByRootNodeId = GetDomains().GroupBy(x => x.RootNodeId);

return (
from domainGroup in domainsByRootNodeId
let content = _contentService.GetById(domainGroup.First().RootNodeId)
where content is { Trashed: false }
orderby content.Id
select RedirectRootNode.GetFromContent(content, domainGroup)
).ToArray();
}

/// <summary>
/// Returns an array of all rode nodes configured in Umbraco.
/// </summary>
/// <param name="user">An <see cref="IUser"/> with potential root node access restrictions.</param>
/// <returns>An array of <see cref="RedirectRootNode"/> representing the root nodes the user has access to.</returns>
public RedirectRootNode[] GetRootNodes(IUser user)
{
var rootNodes = GetRootNodes();
HashSet<int> rootNodeIds = new();

if (user.StartContentIds != null)
{
foreach (var rootNodeId in user.StartContentIds)
{
rootNodeIds.Add(rootNodeId);
}
}

foreach (var group in user.Groups)
{
if (group.StartContentId != null)
{
rootNodeIds.Add(group.StartContentId.Value);
}
}

return rootNodes.Where(rootNode => rootNodeIds.Any(x => rootNode.Path.Contains(x))).ToArray();
Copy link
Member

Choose a reason for hiding this comment

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

This is still a breaking change as you're introducing a new method. It's probably not very likely, but developers may choose to write their own implementation of the IRedirectsService interface, and if they do, they'll now be missing the GetRootNodes method.

C# 8 introduces default interface methods. So as the method doesn't use any dependencies, you should be able to move the method body to the interface instead. This ensures that we can add a new method without it being a breaking change.

Hope that makes sense 😉

}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@

// Any filters?
if (vm.filters.rootNode && vm.filters.rootNode.key) {
args.rootNodeKey = vm.filters.rootNode.key;
args.rootNodeKeys = vm.filters.rootNode.key;
vm.activeFilters++;
}

Expand Down