diff --git a/src/Umbraco.Core/Manifest/ManifestContentAppDefinition.cs b/src/Umbraco.Core/Manifest/ManifestContentAppDefinition.cs index 7b1d311930..3d4b24d359 100644 --- a/src/Umbraco.Core/Manifest/ManifestContentAppDefinition.cs +++ b/src/Umbraco.Core/Manifest/ManifestContentAppDefinition.cs @@ -7,8 +7,6 @@ using Umbraco.Core.IO; using Umbraco.Core.Models; using Umbraco.Core.Models.ContentEditing; using Umbraco.Core.Models.Membership; -using Umbraco.Core.Services; - namespace Umbraco.Core.Manifest { @@ -106,43 +104,28 @@ namespace Umbraco.Core.Manifest } var rules = _showRules ?? (_showRules = ShowRule.Parse(Show).ToArray()); + var userGroupsList = userGroups.ToList(); - // if no 'show' is specified, then always display the content app - if (rules.Length > 0) + var okRole = false; + var hasRole = false; + var okType = false; + var hasType = false; + + foreach (var rule in rules) { - var ok = false; - - //if any role specific rules, deal with them first. - if (rules.Where(x => x.PartA.InvariantEquals("role")).Any()) + if (rule.PartA.InvariantEquals("role")) { - foreach (var rule in rules) - { - if (rule.PartA.InvariantEquals("role")) - { - foreach (var group in userGroups) - { - if (rule.Matches(rule.PartA, group.Alias)) - { - ok = rule.Show; - break; - } - } - } - } - // if a role entry, don't let anything else override it. - if (!ok) - { - return null; - } - } - else - { - // else iterate over each entry to check for show/hide rules. - foreach (var rule in rules) - { + // if roles have been ok-ed already, skip the rule + if (okRole) + continue; - // if the entry does not apply, skip it - if (!rule.Matches(partA, partB)) + // remember we have role rules + hasRole = true; + + foreach (var group in userGroupsList) + { + // if the entry does not apply, skip + if (!rule.Matches("role", group.Alias)) continue; // if the entry applies, @@ -150,16 +133,41 @@ namespace Umbraco.Core.Manifest if (!rule.Show) return null; - // else break - ok to display - ok = true; + // else ok to display, remember roles are ok, break from userGroupsList + okRole = rule.Show; break; } } - // when 'show' is specified, default is to *not* show the content app - if (!ok) - return null; + else // it is a type rule + { + // if type has been ok-ed already, skip the rule + if (okType) + continue; + + // remember we have type rules + hasType = true; + + // if the entry does not apply, skip it + if (!rule.Matches(partA, partB)) + continue; + + // if the entry applies, + // if it's an exclude entry, exit, do not display the content app + if (!rule.Show) + return null; + + // else ok to display, remember type rules are ok + okType = true; + } } + // if roles rules are specified but not ok, + // or if type roles are specified but not ok, + // cannot display the content app + if ((hasRole && !okRole) || (hasType && !okType)) + return null; + + // else // content app can be displayed return _app ?? (_app = new ContentApp { diff --git a/src/Umbraco.Tests/Manifest/ManifestContentAppTests.cs b/src/Umbraco.Tests/Manifest/ManifestContentAppTests.cs new file mode 100644 index 0000000000..eed0919149 --- /dev/null +++ b/src/Umbraco.Tests/Manifest/ManifestContentAppTests.cs @@ -0,0 +1,77 @@ +using System; +using System.Linq; +using Moq; +using Newtonsoft.Json; +using NUnit.Framework; +using Umbraco.Core.Manifest; +using Umbraco.Core.Models; +using Umbraco.Core.Models.Membership; + +namespace Umbraco.Tests.Manifest +{ + [TestFixture] + public class ManifestContentAppTests + { + [Test] + public void Test() + { + var contentType = Mock.Of(); + Mock.Get(contentType).Setup(x => x.Alias).Returns("type1"); + var content = Mock.Of(); + Mock.Get(content).Setup(x => x.ContentType).Returns(contentType); + + var group1 = Mock.Of(); + Mock.Get(group1).Setup(x => x.Alias).Returns("group1"); + var group2 = Mock.Of(); + Mock.Get(group2).Setup(x => x.Alias).Returns("group2"); + + // no rule = ok + AssertDefinition(content, true, Array.Empty(), new [] { group1, group2 }); + + // wildcards = ok + AssertDefinition(content, true, new [] { "+content/*" }, new [] { group1, group2 }); + AssertDefinition(content, false, new[] { "+media/*" }, new [] { group1, group2 }); + + // explicitly enabling / disabling + AssertDefinition(content, true, new[] { "+content/type1" }, new [] { group1, group2 }); + AssertDefinition(content, false, new[] { "-content/type1" }, new [] { group1, group2 }); + + // when there are type rules, failing to approve the type = no app + AssertDefinition(content, false, new[] { "+content/type2" }, new [] { group1, group2 }); + AssertDefinition(content, false, new[] { "+media/type1" }, new [] { group1, group2 }); + + // can have multiple rule, first one that matches = end + AssertDefinition(content, false, new[] { "-content/type1", "+content/*" }, new [] { group1, group2 }); + AssertDefinition(content, true, new[] { "-content/type2", "+content/*" }, new [] { group1, group2 }); + AssertDefinition(content, true, new[] { "+content/*", "-content/type1" }, new [] { group1, group2 }); + + // when there are role rules, failing to approve a role = no app + AssertDefinition(content, false, new[] { "+role/group33" }, new [] { group1, group2 }); + + // wildcards = ok + AssertDefinition(content, true, new[] { "+role/*" }, new [] { group1, group2 }); + + // explicitly enabling / disabling + AssertDefinition(content, true, new[] { "+role/group1" }, new [] { group1, group2 }); + AssertDefinition(content, false, new[] { "-role/group1" }, new [] { group1, group2 }); + + // can have multiple rule, first one that matches = end + AssertDefinition(content, true, new[] { "+role/group1", "-role/group2" }, new [] { group1, group2 }); + + // mixed type and role rules, both are evaluated and need to match + AssertDefinition(content, true, new[] { "+role/group1", "+content/type1" }, new [] { group1, group2 }); + AssertDefinition(content, false, new[] { "+role/group1", "+content/type2" }, new [] { group1, group2 }); + AssertDefinition(content, false, new[] { "+role/group33", "+content/type1" }, new [] { group1, group2 }); + } + + private void AssertDefinition(object source, bool expected, string[] show, IReadOnlyUserGroup[] groups) + { + var definition = JsonConvert.DeserializeObject("{" + (show.Length == 0 ? "" : " \"show\": [" + string.Join(",", show.Select(x => "\"" + x + "\"")) + "] ") + "}"); + var app = definition.GetContentAppFor(source, groups); + if (expected) + Assert.IsNotNull(app); + else + Assert.IsNull(app); + } + } +} diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 61ae537529..04bccc8bcb 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -119,6 +119,7 @@ +