diff --git a/src/Orchard.Core.Tests/Common/Providers/CommonAspectProviderTests.cs b/src/Orchard.Core.Tests/Common/Providers/CommonAspectProviderTests.cs index f32b0b9ec..294b75f9b 100644 --- a/src/Orchard.Core.Tests/Common/Providers/CommonAspectProviderTests.cs +++ b/src/Orchard.Core.Tests/Common/Providers/CommonAspectProviderTests.cs @@ -6,13 +6,18 @@ using JetBrains.Annotations; using Moq; using NUnit.Framework; using Orchard.ContentManagement.Aspects; +using Orchard.Core.Common; using Orchard.Core.Common.Handlers; using Orchard.Core.Common.Models; using Orchard.ContentManagement; using Orchard.ContentManagement.Handlers; using Orchard.ContentManagement.Records; +using Orchard.Localization; using Orchard.Security; using Orchard.Tests.Modules; +using Orchard.Mvc.ViewModels; +using Orchard.Core.Common.ViewModels; +using System.Web.Mvc; namespace Orchard.Core.Tests.Common.Providers { [TestFixture] @@ -29,6 +34,7 @@ namespace Orchard.Core.Tests.Common.Providers { _authn = new Mock(); _authz = new Mock(); _membership = new Mock(); + builder.RegisterInstance(_authn.Object); builder.RegisterInstance(_authz.Object); builder.RegisterInstance(_membership.Object); @@ -73,7 +79,83 @@ namespace Orchard.Core.Tests.Common.Providers { } [Test] - public void PublishingShouldSetPublishUtc() { + public void PublishingShouldFailIfOwnerIsUnknown() + { + var contentManager = _container.Resolve(); + var updateModel = new Mock(); + + var user = contentManager.New("user"); + _authn.Setup(x => x.GetAuthenticatedUser()).Returns(user); + + var createUtc = _clock.UtcNow; + var item = contentManager.Create("test-item", VersionOptions.Draft, init => { }); + var viewModel = new OwnerEditorViewModel() { Owner = "user" }; + updateModel.Setup(x => x.TryUpdateModel(viewModel, "", null, null)).Returns(true); + contentManager.UpdateEditorModel(item.ContentItem, updateModel.Object); + } + + class UpdatModelStub : IUpdateModel { + + ModelStateDictionary _modelState = new ModelStateDictionary(); + + public ModelStateDictionary ModelErrors + { + get { return _modelState; } + } + + public string Owner { get; set; } + + public bool TryUpdateModel(TModel model, string prefix, string[] includeProperties, string[] excludeProperties) where TModel : class { + (model as OwnerEditorViewModel).Owner = Owner; + return true; + } + + public void AddModelError(string key, LocalizedString errorMessage) { + _modelState.AddModelError(key, errorMessage.ToString()); + } + } + + [Test] + public void PublishingShouldNotThrowExceptionIfOwnerIsNull() + { + var contentManager = _container.Resolve(); + + var item = contentManager.Create("test-item", VersionOptions.Draft, init => { }); + + var user = contentManager.New("user"); + _authn.Setup(x => x.GetAuthenticatedUser()).Returns(user); + _authz.Setup(x => x.TryCheckAccess(Permissions.ChangeOwner, user, item)).Returns(true); + + item.Owner = user; + + var updater = new UpdatModelStub() { Owner = null }; + + contentManager.UpdateEditorModel(item.ContentItem, updater); + } + + [Test] + public void PublishingShouldFailIfOwnerIsEmpty() + { + var contentManager = _container.Resolve(); + + var item = contentManager.Create("test-item", VersionOptions.Draft, init => { }); + + var user = contentManager.New("user"); + _authn.Setup(x => x.GetAuthenticatedUser()).Returns(user); + _authz.Setup(x => x.TryCheckAccess(Permissions.ChangeOwner, user, item)).Returns(true); + + item.Owner = user; + + var updater = new UpdatModelStub() {Owner = ""}; + + contentManager.UpdateEditorModel(item.ContentItem, updater); + + Assert.That(updater.ModelErrors.ContainsKey("CommonAspect.Owner"), Is.True); + } + + [Test] + public void PublishingShouldSetPublishUtc() + { var contentManager = _container.Resolve(); var createUtc = _clock.UtcNow; @@ -81,7 +163,7 @@ namespace Orchard.Core.Tests.Common.Providers { Assert.That(item.CreatedUtc, Is.EqualTo(createUtc)); Assert.That(item.PublishedUtc, Is.Null); - + _clock.Advance(TimeSpan.FromMinutes(1)); var publishUtc = _clock.UtcNow; @@ -91,6 +173,7 @@ namespace Orchard.Core.Tests.Common.Providers { Assert.That(item.PublishedUtc, Is.EqualTo(publishUtc)); } + [Test] public void VersioningItemShouldCreatedAndPublishedUtcValuesPerVersion() { var contentManager = _container.Resolve(); diff --git a/src/Orchard.Core.Tests/Common/Services/RoutableServiceTests.cs b/src/Orchard.Core.Tests/Common/Services/RoutableServiceTests.cs index 5a3588b21..1a3b18f47 100644 --- a/src/Orchard.Core.Tests/Common/Services/RoutableServiceTests.cs +++ b/src/Orchard.Core.Tests/Common/Services/RoutableServiceTests.cs @@ -10,6 +10,10 @@ using Orchard.ContentManagement.Records; using Orchard.Core.Common.Models; using Orchard.Core.Common.Services; using Orchard.Tests.Modules; +using Orchard.Core.Common.Handlers; +using System.Web.Mvc; +using System.Web.Routing; +using Orchard.Tests.Stubs; namespace Orchard.Core.Tests.Common.Services { [TestFixture] @@ -23,7 +27,13 @@ namespace Orchard.Core.Tests.Common.Services { public override void Register(ContainerBuilder builder) { builder.RegisterType().As(); builder.RegisterType().As(); + builder.RegisterType().As(); builder.RegisterType().As(); + + builder.RegisterType().As(); + builder.RegisterInstance(new UrlHelper(new RequestContext(new StubHttpContext("~/"), new RouteData()))).As(); + builder.RegisterType().As(); + } private IRoutableService _routableService; @@ -42,6 +52,24 @@ namespace Orchard.Core.Tests.Common.Services { Assert.That(thing.Slug, Is.EqualTo("please-do-not-use-any-of-the-following-characters-in-your-slugs-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"")); } + [Test] + public void EmptySlugsShouldBeConsideredValid() { + // so that automatic generation on Publish occurs + Assert.That(_routableService.IsSlugValid(null), Is.True); + Assert.That(_routableService.IsSlugValid(String.Empty), Is.True); + Assert.That(_routableService.IsSlugValid(" "), Is.True); + } + + [Test] + public void InvalidCharacterShouldBeRefusedInSlugs() { + Assert.That(_routableService.IsSlugValid("aaaa-_aaaa"), Is.True); + + foreach (var c in @"/:?#[]@!$&'()*+,;= ") { + Assert.That(_routableService.IsSlugValid("a" + c + "b"), Is.False); + } + } + + [Test] public void VeryLongStringTruncatedTo1000Chars() { var contentManager = _container.Resolve(); @@ -111,6 +139,47 @@ namespace Orchard.Core.Tests.Common.Services { Assert.That(thing.Slug, Is.EqualTo("this-is-some-interesting-title")); } + [Test] + public void GeneratedSlugsShouldBeUniqueAmongContentType() + { + var contentManager = _container.Resolve(); + + var thing1 = contentManager.Create(ThingDriver.ContentType.Name, t => + { + t.As().Record = new RoutableRecord(); + t.Title = "This Is Some Interesting Title"; + }); + + var thing2 = contentManager.Create(ThingDriver.ContentType.Name , t => + { + t.As().Record = new RoutableRecord(); + t.Title = "This Is Some Interesting Title"; + }); + + Assert.AreNotEqual(thing1.Slug, thing2.Slug); + } + + [Test] + public void SlugsCanBeDuplicatedAccrossContentTypes() + { + var contentManager = _container.Resolve(); + + var thing = contentManager.Create(ThingDriver.ContentType.Name, t => + { + t.As().Record = new RoutableRecord(); + t.Title = "This Is Some Interesting Title"; + }); + + var stuff = contentManager.Create(StuffDriver.ContentType.Name, s => + { + s.As().Record = new RoutableRecord(); + s.Title = "This Is Some Interesting Title"; + }); + + Assert.AreEqual(thing.Slug, stuff.Slug); + } + + protected override IEnumerable DatabaseTypes { get { return new[] { @@ -155,5 +224,43 @@ namespace Orchard.Core.Tests.Common.Services { DisplayName = "Thing" }; } + + [UsedImplicitly] + public class StuffHandler : ContentHandler + { + public StuffHandler() + { + Filters.Add(new ActivatingFilter(StuffDriver.ContentType.Name)); + Filters.Add(new ActivatingFilter>(StuffDriver.ContentType.Name)); + Filters.Add(new ActivatingFilter(StuffDriver.ContentType.Name)); + Filters.Add(new ActivatingFilter(StuffDriver.ContentType.Name)); + } + } + + public class Stuff : ContentPart + { + public int Id { get { return ContentItem.Id; } } + + public string Title + { + get { return this.As().Title; } + set { this.As().Title = value; } + } + + public string Slug + { + get { return this.As().Slug; } + set { this.As().Slug = value; } + } + } + + public class StuffDriver : ContentItemDriver + { + public readonly static ContentType ContentType = new ContentType + { + Name = "stuff", + DisplayName = "Stuff" + }; + } } } \ No newline at end of file diff --git a/src/Orchard.Core.Tests/Orchard.Core.Tests.csproj b/src/Orchard.Core.Tests/Orchard.Core.Tests.csproj index 490fa4e25..7eb333c40 100644 --- a/src/Orchard.Core.Tests/Orchard.Core.Tests.csproj +++ b/src/Orchard.Core.Tests/Orchard.Core.Tests.csproj @@ -60,6 +60,9 @@ ..\..\lib\sqlite\System.Data.SQLite.DLL True + + 3.5 + False ..\..\lib\aspnetmvc\System.Web.Mvc.dll diff --git a/src/Orchard.Web/Core/Common/Drivers/RoutableDriver.cs b/src/Orchard.Web/Core/Common/Drivers/RoutableDriver.cs index 9261c0a81..1abaf162e 100644 --- a/src/Orchard.Web/Core/Common/Drivers/RoutableDriver.cs +++ b/src/Orchard.Web/Core/Common/Drivers/RoutableDriver.cs @@ -3,16 +3,31 @@ using Orchard.ContentManagement; using Orchard.ContentManagement.Drivers; using Orchard.Core.Common.Models; using Orchard.Core.Common.ViewModels; +using Orchard.Core.Common.Services; +using Orchard.Localization; +using Orchard.UI.Notify; namespace Orchard.Core.Common.Drivers { [UsedImplicitly] public class Routable : ContentPartDriver { private const string TemplateName = "Parts/Common.Routable"; + private readonly IOrchardServices _services; + private readonly IRoutableService _routableService; + private Localizer T { get; set; } + protected override string Prefix { get { return "Routable"; } } + public Routable(IOrchardServices services, IRoutableService routableService) + { + _services = services; + _routableService = routableService; + + T = NullLocalizer.Instance; + } + protected override DriverResult Editor(RoutableAspect part) { var model = new RoutableEditorViewModel { Prefix = Prefix, RoutableAspect = part }; return ContentPartTemplate(model, TemplateName, Prefix).Location("primary", "before.5"); @@ -21,7 +36,19 @@ namespace Orchard.Core.Common.Drivers { protected override DriverResult Editor(RoutableAspect part, IUpdateModel updater) { var model = new RoutableEditorViewModel { Prefix = Prefix, RoutableAspect = part }; updater.TryUpdateModel(model, Prefix, null, null); + + if (!_routableService.IsSlugValid(part.Slug)){ + updater.AddModelError("Routable.Slug", T("Please do not use any of the following characters in your slugs: \"/\", \":\", \"?\", \"#\", \"[\", \"]\", \"@\", \"!\", \"$\", \"&\", \"'\", \"(\", \")\", \"*\", \"+\", \",\", \";\", \"=\". No spaces are allowed (please use dashes or underscores instead).").ToString()); + } + + string originalSlug = part.Slug; + if(!_routableService.ProcessSlug(part)) { + _services.Notifier.Warning(T("Slugs in conflict. \"{0}\" is already set for a previously created {2} so now it has the slug \"{1}\"", + originalSlug, part.Slug, part.ContentItem.ContentType)); + } + return ContentPartTemplate(model, TemplateName, Prefix).Location("primary", "before.5"); } + } } \ No newline at end of file diff --git a/src/Orchard.Web/Core/Common/Handlers/CommonAspectHandler.cs b/src/Orchard.Web/Core/Common/Handlers/CommonAspectHandler.cs index 4ad9f4337..a2f3d4366 100644 --- a/src/Orchard.Web/Core/Common/Handlers/CommonAspectHandler.cs +++ b/src/Orchard.Web/Core/Common/Handlers/CommonAspectHandler.cs @@ -172,7 +172,7 @@ namespace Orchard.Core.Common.Handlers { var priorOwner = viewModel.Owner; context.Updater.TryUpdateModel(viewModel, "CommonAspect", null, null); - if (viewModel.Owner != priorOwner) { + if (viewModel.Owner != null && viewModel.Owner != priorOwner) { var newOwner = _membershipService.GetUser(viewModel.Owner); if (newOwner == null) { context.Updater.AddModelError("CommonAspect.Owner", T("Invalid user name")); @@ -181,6 +181,7 @@ namespace Orchard.Core.Common.Handlers { instance.Owner = newOwner; } } + context.AddEditor(new TemplateViewModel(viewModel, "CommonAspect") { TemplateName = "Parts/Common.Owner", ZoneName = "primary", Position = "999" }); } } diff --git a/src/Orchard.Web/Core/Common/Handlers/RoutableAspectHandler.cs b/src/Orchard.Web/Core/Common/Handlers/RoutableAspectHandler.cs index b902fb2c6..8f654378b 100644 --- a/src/Orchard.Web/Core/Common/Handlers/RoutableAspectHandler.cs +++ b/src/Orchard.Web/Core/Common/Handlers/RoutableAspectHandler.cs @@ -7,6 +7,7 @@ using Orchard.ContentManagement; using Orchard.ContentManagement.Drivers; using Orchard.ContentManagement.Handlers; using Orchard.Core.Common.Models; +using Orchard.Core.Common.Services; using Orchard.Data; namespace Orchard.Core.Common.Handlers { @@ -14,7 +15,7 @@ namespace Orchard.Core.Common.Handlers { public class RoutableAspectHandler : ContentHandler { private readonly IEnumerable _contentItemDrivers; - public RoutableAspectHandler(IRepository repository, IEnumerable contentItemDrivers, UrlHelper urlHelper) { + public RoutableAspectHandler(IRepository repository, IEnumerable contentItemDrivers, IRoutableService routableService, UrlHelper urlHelper) { _contentItemDrivers = contentItemDrivers; Filters.Add(StorageFilter.For(repository)); @@ -32,6 +33,9 @@ namespace Orchard.Core.Common.Handlers { routable.ContentItemBasePath = url; }); + + OnCreated((context, ra) => routableService.ProcessSlug(ra)); + } private static RouteValueDictionary GetRouteValues(IContentItemDriver driver, ContentItem contentItem) { diff --git a/src/Orchard.Web/Core/Common/Services/IRoutableService.cs b/src/Orchard.Web/Core/Common/Services/IRoutableService.cs index 2780a4789..65f0fac1a 100644 --- a/src/Orchard.Web/Core/Common/Services/IRoutableService.cs +++ b/src/Orchard.Web/Core/Common/Services/IRoutableService.cs @@ -7,5 +7,22 @@ namespace Orchard.Core.Common.Services { void FillSlug(TModel model) where TModel : RoutableAspect; void FillSlug(TModel model, Func generateSlug) where TModel : RoutableAspect; string GenerateUniqueSlug(string slugCandidate, IEnumerable existingSlugs); + + /// + /// Returns any content item of the specified content type with similar slugs + /// + string[] GetSimilarSlugs(string contentType, string slug); + + /// + /// Validates the given slug + /// + bool IsSlugValid(string slug); + + /// + /// Defines the slug of a RoutableAspect and validate its unicity + /// + /// True if the slug has been created, False if a conflict occured + bool ProcessSlug(RoutableAspect part); + } } \ No newline at end of file diff --git a/src/Orchard.Web/Core/Common/Services/RoutableService.cs b/src/Orchard.Web/Core/Common/Services/RoutableService.cs index f0f737444..cfddb58b5 100644 --- a/src/Orchard.Web/Core/Common/Services/RoutableService.cs +++ b/src/Orchard.Web/Core/Common/Services/RoutableService.cs @@ -4,10 +4,19 @@ using System.Linq; using System.Text.RegularExpressions; using JetBrains.Annotations; using Orchard.Core.Common.Models; +using Orchard.ContentManagement; +using Orchard.Localization; +using Orchard.UI.Notify; namespace Orchard.Core.Common.Services { [UsedImplicitly] public class RoutableService : IRoutableService { + private readonly IContentManager _contentManager; + + public RoutableService(IContentManager contentManager) { + _contentManager = contentManager; + } + public void FillSlug(TModel model) where TModel : RoutableAspect { if (!string.IsNullOrEmpty(model.Slug) || string.IsNullOrEmpty(model.Title)) return; @@ -31,8 +40,6 @@ namespace Orchard.Core.Common.Services { model.Slug = generateSlug(model.Title).ToLowerInvariant(); } - - public string GenerateUniqueSlug(string slugCandidate, IEnumerable existingSlugs) { if (existingSlugs == null || !existingSlugs.Contains(slugCandidate)) return slugCandidate; @@ -55,5 +62,46 @@ namespace Orchard.Core.Common.Services { ? (int?)++v : null; } + + public string[] GetSimilarSlugs(string contentType, string slug) + { + return + _contentManager.Query(contentType).Join() + .List() + .Select(i => i.As().Slug) + .Where(rr => rr.StartsWith(slug, StringComparison.OrdinalIgnoreCase)) // todo: for some reason the filter doesn't work within the query, even without StringComparison or StartsWith + .ToArray(); + } + + public bool IsSlugValid(string slug) { + // see http://tools.ietf.org/html/rfc3987 for prohibited chars + return slug == null || String.IsNullOrEmpty(slug.Trim()) || Regex.IsMatch(slug, @"^[^/:?#\[\]@!$&'()*+,;=\s]+$"); + } + + public bool ProcessSlug(RoutableAspect part) + { + FillSlug(part); + + if (string.IsNullOrEmpty(part.Slug)) + { + return true; + } + + var slugsLikeThis = GetSimilarSlugs(part.ContentItem.ContentType, part.Slug); + + //todo: (heskew) need better messages + if (slugsLikeThis.Length > 0) + { + var originalSlug = part.Slug; + //todo: (heskew) make auto-uniqueness optional + part.Slug = GenerateUniqueSlug(part.Slug, slugsLikeThis); + + if (originalSlug != part.Slug) { + return false; + } + } + + return true; + } } } \ No newline at end of file diff --git a/src/Orchard.Web/Modules/Orchard.Blogs/Drivers/BlogDriver.cs b/src/Orchard.Web/Modules/Orchard.Blogs/Drivers/BlogDriver.cs index 61485ccf5..99126d397 100644 --- a/src/Orchard.Web/Modules/Orchard.Blogs/Drivers/BlogDriver.cs +++ b/src/Orchard.Web/Modules/Orchard.Blogs/Drivers/BlogDriver.cs @@ -27,14 +27,12 @@ namespace Orchard.Blogs.Drivers { private readonly IContentManager _contentManager; private readonly IBlogService _blogService; private readonly IBlogPostService _blogPostService; - private readonly IRoutableService _routableService; - public BlogDriver(IOrchardServices services, IContentManager contentManager, IBlogService blogService, IBlogPostService blogPostService, IRoutableService routableService) { + public BlogDriver(IOrchardServices services, IContentManager contentManager, IBlogService blogService, IBlogPostService blogPostService) { Services = services; _contentManager = contentManager; _blogService = blogService; _blogPostService = blogPostService; - _routableService = routableService; T = NullLocalizer.Instance; } @@ -97,47 +95,9 @@ namespace Orchard.Blogs.Drivers { protected override DriverResult Editor(Blog blog, IUpdateModel updater) { updater.TryUpdateModel(blog, Prefix, null, null); - //todo: (heskew) something better needs to be done with this...still feels shoehorned in here - ProcessSlug(blog, updater); - return Combined( ContentItemTemplate("Items/Blogs.Blog"), ContentPartTemplate(blog, "Parts/Blogs.Blog.Fields").Location("primary", "1")); } - - private void ProcessSlug(Blog blog, IUpdateModel updater) { - _routableService.FillSlug(blog.As()); - - - if (string.IsNullOrEmpty(blog.Slug)) { - return; - - // OR - - // updater.AddModelError("Routable.Slug", T("The slug is required.").ToString()); - // return; - } - - - if (!Regex.IsMatch(blog.Slug, @"^[^/:?#\[\]@!$&'()*+,;=\s]+$")) { - //todo: (heskew) get rid of the hard-coded prefix - updater.AddModelError("Routable.Slug", T("Please do not use any of the following characters in your slugs: \"/\", \":\", \"?\", \"#\", \"[\", \"]\", \"@\", \"!\", \"$\", \"&\", \"'\", \"(\", \")\", \"*\", \"+\", \",\", \";\", \"=\". No spaces are allowed (please use dashes or underscores instead).").ToString()); - } - - var slugsLikeThis = _blogService.Get().Where( - b => b.Slug.StartsWith(blog.Slug, StringComparison.OrdinalIgnoreCase) && - b.Id != blog.Id).Select(b => b.Slug); - - //todo: (heskew) need better messages - if (slugsLikeThis.Count() > 0) { - var originalSlug = blog.Slug; - //todo: (heskew) make auto-uniqueness optional - blog.Slug = _routableService.GenerateUniqueSlug(blog.Slug, slugsLikeThis); - - if (originalSlug != blog.Slug) - Services.Notifier.Warning(T("Slugs in conflict. \"{0}\" is already set for a previously created blog so this blog now has the slug \"{1}\"", - originalSlug, blog.Slug)); - } - } } } \ No newline at end of file diff --git a/src/Orchard.Web/Modules/Orchard.Blogs/Drivers/BlogPostDriver.cs b/src/Orchard.Web/Modules/Orchard.Blogs/Drivers/BlogPostDriver.cs index adec3f1ff..8ac6ab501 100644 --- a/src/Orchard.Web/Modules/Orchard.Blogs/Drivers/BlogPostDriver.cs +++ b/src/Orchard.Web/Modules/Orchard.Blogs/Drivers/BlogPostDriver.cs @@ -1,19 +1,15 @@ using System; -using System.Linq; -using System.Text.RegularExpressions; using System.Web.Routing; using JetBrains.Annotations; using Orchard.Blogs.Models; using Orchard.Blogs.Services; using Orchard.ContentManagement; using Orchard.ContentManagement.Drivers; -using Orchard.Core.Common.Models; using Orchard.Core.Common.Services; using Orchard.Localization; -using Orchard.UI.Notify; namespace Orchard.Blogs.Drivers { - [UsedImplicitly] + [UsedImplicitly] public class BlogPostDriver : ContentItemDriver { public IOrchardServices Services { get; set; } private readonly IBlogPostService _blogPostService; @@ -78,54 +74,11 @@ namespace Orchard.Blogs.Drivers { protected override DriverResult Editor(BlogPost post, IUpdateModel updater) { updater.TryUpdateModel(post, Prefix, null, null); - //todo: (heskew) something better needs to be done with this...still feels shoehorned in here - ProcessSlug(post, updater); - DateTime scheduled; if (DateTime.TryParse(string.Format("{0} {1}", post.ScheduledPublishUtcDate, post.ScheduledPublishUtcTime), out scheduled)) post.ScheduledPublishUtc = scheduled; return Editor(post); } - - private void ProcessSlug(BlogPost post, IUpdateModel updater) { - _routableService.FillSlug(post.As()); - - if (string.IsNullOrEmpty(post.Slug)) { - return; - - // OR - - //updater.AddModelError("Routable.Slug", T("The slug is required.").ToString()); - //return; - } - - if (!Regex.IsMatch(post.Slug, @"^[^/:?#\[\]@!$&'()*+,;=\s]+$")) { - //todo: (heskew) get rid of the hard-coded prefix - updater.AddModelError("Routable.Slug", T("Please do not use any of the following characters in your slugs: \"/\", \":\", \"?\", \"#\", \"[\", \"]\", \"@\", \"!\", \"$\", \"&\", \"'\", \"(\", \")\", \"*\", \"+\", \",\", \";\", \"=\". No spaces are allowed (please use dashes or underscores instead).").ToString()); - return; - } - - var slugsLikeThis = _blogPostService.Get(post.Blog, VersionOptions.Published).Where( - p => p.Slug.StartsWith(post.Slug, StringComparison.OrdinalIgnoreCase) && - p.Id != post.Id).Select(p => p.Slug); - - //todo: (heskew) need better messages - if (slugsLikeThis.Count() > 0) { - //todo: (heskew) need better messages - Services.Notifier.Warning(T("A different blog post is already published with this same slug.")); - - if (post.ContentItem.VersionRecord == null || post.ContentItem.VersionRecord.Published) - { - var originalSlug = post.Slug; - //todo: (heskew) make auto-uniqueness optional - post.Slug = _routableService.GenerateUniqueSlug(post.Slug, slugsLikeThis); - - if (originalSlug != post.Slug) - Services.Notifier.Warning(T("Slugs in conflict. \"{0}\" is already set for a previously created blog post so this post now has the slug \"{1}\"", - originalSlug, post.Slug)); - } - } - } } } \ No newline at end of file diff --git a/src/Orchard.Web/Modules/Orchard.Blogs/Handlers/BlogPostHandler.cs b/src/Orchard.Web/Modules/Orchard.Blogs/Handlers/BlogPostHandler.cs index 7e7864d3d..e77a70fe4 100644 --- a/src/Orchard.Web/Modules/Orchard.Blogs/Handlers/BlogPostHandler.cs +++ b/src/Orchard.Web/Modules/Orchard.Blogs/Handlers/BlogPostHandler.cs @@ -16,12 +16,10 @@ namespace Orchard.Blogs.Handlers { [UsedImplicitly] public class BlogPostHandler : ContentHandler { private readonly IBlogPostService _blogPostService; - private readonly IRoutableService _routableService; private readonly IOrchardServices _orchardServices; - public BlogPostHandler(IBlogService blogService, IBlogPostService blogPostService, IRoutableService routableService, IOrchardServices orchardServices, RequestContext requestContext) { + public BlogPostHandler(IBlogService blogService, IBlogPostService blogPostService, IOrchardServices orchardServices, RequestContext requestContext) { _blogPostService = blogPostService; - _routableService = routableService; _orchardServices = orchardServices; T = NullLocalizer.Instance; @@ -63,8 +61,6 @@ namespace Orchard.Blogs.Handlers { OnVersioned((context, bp1, bp2) => updateBlogPostCount(bp2.Blog)); OnRemoved((context, bp) => updateBlogPostCount(bp.Blog)); - OnPublished((context, bp) => ProcessSlug(bp)); - OnRemoved( (context, b) => blogPostService.Get(context.ContentItem.As()).ToList().ForEach( @@ -72,23 +68,5 @@ namespace Orchard.Blogs.Handlers { } Localizer T { get; set; } - - private void ProcessSlug(BlogPost post) { - _routableService.FillSlug(post.As()); - - var slugsLikeThis = _blogPostService.Get(post.Blog, VersionOptions.Published).Where( - p => p.Slug.StartsWith(post.Slug, StringComparison.OrdinalIgnoreCase) && - p.Id != post.Id).Select(p => p.Slug); - - //todo: (heskew) need better messages - if (slugsLikeThis.Count() > 0) { - //todo: (heskew) need better messages - var originalSlug = post.Slug; - post.Slug = _routableService.GenerateUniqueSlug(post.Slug, slugsLikeThis); - - if (originalSlug != post.Slug) - _orchardServices.Notifier.Warning(T("A different blog post is already published with this same slug ({0}) so a unique slug ({1}) was generated for this post.", originalSlug, post.Slug)); - } - } } } \ No newline at end of file diff --git a/src/Orchard.Web/Modules/Orchard.Comments/Controllers/CommentController.cs b/src/Orchard.Web/Modules/Orchard.Comments/Controllers/CommentController.cs index b10ed9166..1f9bb75ef 100644 --- a/src/Orchard.Web/Modules/Orchard.Comments/Controllers/CommentController.cs +++ b/src/Orchard.Web/Modules/Orchard.Comments/Controllers/CommentController.cs @@ -9,6 +9,7 @@ using Orchard.Localization; using Orchard.Security; using Orchard.Settings; using Orchard.UI.Notify; +using Orchard.Utility.Extensions; namespace Orchard.Comments.Controllers { public class CommentController : Controller { @@ -42,7 +43,15 @@ namespace Orchard.Comments.Controllers { var viewModel = new CommentsCreateViewModel(); try { - UpdateModel(viewModel); + + // UpdateModel(viewModel); + + if(!TryUpdateModel(viewModel)) { + if (Request.Form["Name"].IsNullOrEmptyTrimmed()) { + _notifier.Error("You must provide a Name in order to comment"); + } + return Redirect(returnUrl); + } var context = new CreateCommentContext { Author = viewModel.Name, @@ -63,7 +72,8 @@ namespace Orchard.Comments.Controllers { } catch (Exception exception) { _notifier.Error(T("Creating Comment failed: " + exception.Message)); - return View(viewModel); + // return View(viewModel); + return Redirect(returnUrl); } } } diff --git a/src/Orchard.Web/Modules/Orchard.Pages/Drivers/PageDriver.cs b/src/Orchard.Web/Modules/Orchard.Pages/Drivers/PageDriver.cs index 32b92971a..c06589e1f 100644 --- a/src/Orchard.Web/Modules/Orchard.Pages/Drivers/PageDriver.cs +++ b/src/Orchard.Web/Modules/Orchard.Pages/Drivers/PageDriver.cs @@ -76,74 +76,11 @@ namespace Orchard.Pages.Drivers { protected override DriverResult Editor(Page page, IUpdateModel updater) { updater.TryUpdateModel(page, Prefix, null, null); - //todo: (heskew) something better needs to be done with this...still feels shoehorned in here - ProcessSlug(page, updater); - DateTime scheduled; if (DateTime.TryParse(string.Format("{0} {1}", page.ScheduledPublishUtcDate, page.ScheduledPublishUtcTime), out scheduled)) page.ScheduledPublishUtc = scheduled; return Editor(page); } - - private void ProcessSlug(Page page, IUpdateModel updater) { - _routableService.FillSlug(page.As(), Slugify); - - if (string.IsNullOrEmpty(page.Slug)) { - return; - - // OR - - //updater.AddModelError("Routable.Slug", T("The slug is required.").ToString()); - //return; - } - - if (!Regex.IsMatch(page.Slug, @"^[^/:?#\[\]@!$&'()*+,;=\s](?(?=/)/[^/:?#\[\]@!$&'()*+,;=\s]|[^:?#\[\]@!$&'()*+,;=\s])*$")) { - updater.AddModelError("Routable.Slug", T("Please do not use any of the following characters in your slugs: \":\", \"?\", \"#\", \"[\", \"]\", \"@\", \"!\", \"$\", \"&\", \"'\", \"(\", \")\", \"*\", \"+\", \",\", \";\", \"=\". No spaces are allowed (please use dashes or underscores instead).").ToString()); - return; - } - - var slugsLikeThis = _pageService.Get(PageStatus.Published).Where( - p => p.Slug.StartsWith(page.Slug, StringComparison.OrdinalIgnoreCase) && - p.Id != page.Id).Select(p => p.Slug); - - if (slugsLikeThis.Count() > 0) { - //todo: (heskew) need better messages - Services.Notifier.Warning(T("A different page is already published with this same slug.")); - - if (page.ContentItem.VersionRecord == null || page.ContentItem.VersionRecord.Published) { - var originalSlug = page.Slug; - //todo: (heskew) make auto-uniqueness optional - page.Slug = _routableService.GenerateUniqueSlug(page.Slug, slugsLikeThis); - - //todo: (heskew) need better messages - if (originalSlug != page.Slug) - Services.Notifier.Warning(T("Slugs in conflict. \"{0}\" is already set for a previously published page so this page now has the slug \"{1}\"", - originalSlug, page.Slug)); - } - } - } - - private static string Slugify(string value) - { - if (!string.IsNullOrEmpty(value)) - { - //todo: (heskew) improve - just doing multi-pass regex replaces for now with the simple rules of - // (1) can't begin with a '/', (2) can't have adjacent '/'s and (3) can't have these characters - var startsoffbad = new Regex(@"^[\s/]+"); - var slashhappy = new Regex("/{2,}"); - var dissallowed = new Regex(@"[:?#\[\]@!$&'()*+,;=\s]+"); - - value = startsoffbad.Replace(value, "-"); - value = slashhappy.Replace(value, "/"); - value = dissallowed.Replace(value, "-"); - value = value.Trim('-'); - - if (value.Length > 1000) - value = value.Substring(0, 1000); - } - - return value; - } } } \ No newline at end of file diff --git a/src/Orchard.Web/Modules/Orchard.Pages/Handlers/PageHandler.cs b/src/Orchard.Web/Modules/Orchard.Pages/Handlers/PageHandler.cs index f000eb348..6b3b2e7a9 100644 --- a/src/Orchard.Web/Modules/Orchard.Pages/Handlers/PageHandler.cs +++ b/src/Orchard.Web/Modules/Orchard.Pages/Handlers/PageHandler.cs @@ -15,12 +15,10 @@ namespace Orchard.Pages.Handlers { [UsedImplicitly] public class PageHandler : ContentHandler { private readonly IPageService _pageService; - private readonly IRoutableService _routableService; private readonly IOrchardServices _orchardServices; - public PageHandler(IPageService pageService, IRoutableService routableService, IOrchardServices orchardServices) { + public PageHandler(IPageService pageService, IOrchardServices orchardServices) { _pageService = pageService; - _routableService = routableService; _orchardServices = orchardServices; T = NullLocalizer.Instance; @@ -31,27 +29,8 @@ namespace Orchard.Pages.Handlers { Filters.Add(new ActivatingFilter(PageDriver.ContentType.Name)); OnLoaded((context, p) => p.ScheduledPublishUtc = _pageService.GetScheduledPublishUtc(p)); - OnPublished((context, p) => ProcessSlug(p)); } Localizer T { get; set; } - - private void ProcessSlug(Page page) { - _routableService.FillSlug(page.As()); - - var slugsLikeThis = _pageService.Get(PageStatus.Published).Where( - p => p.Slug.StartsWith(page.Slug, StringComparison.OrdinalIgnoreCase) && - p.Id != page.Id).Select(p => p.Slug); - - //todo: (heskew) need better messages - if (slugsLikeThis.Count() > 0) { - //todo: (heskew) need better messages - var originalSlug = page.Slug; - page.Slug = _routableService.GenerateUniqueSlug(page.Slug, slugsLikeThis); - - if (originalSlug != page.Slug) - _orchardServices.Notifier.Warning(T("A different page is already published with this same slug ({0}) so a unique slug ({1}) was generated for this page.", originalSlug, page.Slug)); - } - } } } \ No newline at end of file diff --git a/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs b/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs index 5975abe20..c4f13ee73 100644 --- a/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs +++ b/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs @@ -1,6 +1,6 @@ using System; using System.Diagnostics.CodeAnalysis; -using System.Globalization; +using Orchard.Localization; using System.Security.Principal; using System.Web.Mvc; using System.Web.Security; @@ -26,9 +26,11 @@ namespace Orchard.Users.Controllers { _membershipService = membershipService; _userService = userService; Logger = NullLogger.Instance; + T = NullLocalizer.Instance; } public ILogger Logger { get; set; } + public Localizer T { get; set; } public ActionResult AccessDenied() { var returnUrl = Request.QueryString["ReturnUrl"]; @@ -55,8 +57,8 @@ namespace Orchard.Users.Controllers { [HttpPost] [SuppressMessage("Microsoft.Design", "CA1054:UriParametersShouldNotBeStrings", Justification = "Needs to take same parameter type as Controller.Redirect()")] - public ActionResult LogOn(string userName, string password, bool rememberMe) { - var user = ValidateLogOn(userName, password); + public ActionResult LogOn(string userNameOrEmail, string password, bool rememberMe) { + var user = ValidateLogOn(userNameOrEmail, password); if (!ModelState.IsValid) { return View("LogOn", new LogOnViewModel {Title = "Log On"}); } @@ -98,7 +100,7 @@ namespace Orchard.Users.Controllers { return Redirect("~/"); } else { - ModelState.AddModelError("_FORM", ErrorCodeToString(/*createStatus*/MembershipCreateStatus.ProviderError)); + ModelState.AddModelError("_FORM", T(ErrorCodeToString(/*createStatus*/MembershipCreateStatus.ProviderError))); } } @@ -133,12 +135,12 @@ namespace Orchard.Users.Controllers { } else { ModelState.AddModelError("_FORM", - "The current password is incorrect or the new password is invalid."); + T("The current password is incorrect or the new password is invalid.")); return ChangePassword(); } } catch { - ModelState.AddModelError("_FORM", "The current password is incorrect or the new password is invalid."); + ModelState.AddModelError("_FORM", T("The current password is incorrect or the new password is invalid.")); return ChangePassword(); } } @@ -157,32 +159,29 @@ namespace Orchard.Users.Controllers { private bool ValidateChangePassword(string currentPassword, string newPassword, string confirmPassword) { if (String.IsNullOrEmpty(currentPassword)) { - ModelState.AddModelError("currentPassword", "You must specify a current password."); + ModelState.AddModelError("currentPassword", T("You must specify a current password.")); } if (newPassword == null || newPassword.Length < MinPasswordLength) { - ModelState.AddModelError("newPassword", - String.Format(CultureInfo.CurrentCulture, - "You must specify a new password of {0} or more characters.", - MinPasswordLength)); + ModelState.AddModelError("newPassword", T("You must specify a new password of {0} or more characters.", MinPasswordLength)); } if (!String.Equals(newPassword, confirmPassword, StringComparison.Ordinal)) { - ModelState.AddModelError("_FORM", "The new password and confirmation password do not match."); + ModelState.AddModelError("_FORM", T("The new password and confirmation password do not match.")); } return ModelState.IsValid; } - private IUser ValidateLogOn(string userName, string password) { - if (String.IsNullOrEmpty(userName)) { - ModelState.AddModelError("username", "You must specify a username."); + private IUser ValidateLogOn(string userNameOrEmail, string password) { + if (String.IsNullOrEmpty(userNameOrEmail)) { + ModelState.AddModelError("userNameOrEmail", T("You must specify a username or e-mail.")); } if (String.IsNullOrEmpty(password)) { - ModelState.AddModelError("password", "You must specify a password."); + ModelState.AddModelError("password", T("You must specify a password.")); } - var user = _membershipService.ValidateUser(userName, password); + var user = _membershipService.ValidateUser(userNameOrEmail, password); if (user == null) { - ModelState.AddModelError("_FORM", "The username or password provided is incorrect."); + ModelState.AddModelError("_FORM", T("The username or e-mail or password provided is incorrect.")); } return user; @@ -190,23 +189,20 @@ namespace Orchard.Users.Controllers { private bool ValidateRegistration(string userName, string email, string password, string confirmPassword) { if (String.IsNullOrEmpty(userName)) { - ModelState.AddModelError("username", "You must specify a username."); + ModelState.AddModelError("username", T("You must specify a username.")); } if (String.IsNullOrEmpty(email)) { - ModelState.AddModelError("email", "You must specify an email address."); + ModelState.AddModelError("email", T("You must specify an email address.")); } string userUnicityMessage = _userService.VerifyUserUnicity(userName, email); if (userUnicityMessage != null) { - ModelState.AddModelError("userExists", userUnicityMessage); + ModelState.AddModelError("userExists", T(userUnicityMessage)); } if (password == null || password.Length < MinPasswordLength) { - ModelState.AddModelError("password", - String.Format(CultureInfo.CurrentCulture, - "You must specify a password of {0} or more characters.", - MinPasswordLength)); + ModelState.AddModelError("password", T("You must specify a password of {0} or more characters.", MinPasswordLength)); } if (!String.Equals(password, confirmPassword, StringComparison.Ordinal)) { - ModelState.AddModelError("_FORM", "The new password and confirmation password do not match."); + ModelState.AddModelError("_FORM", T("The new password and confirmation password do not match.")); } return ModelState.IsValid; } diff --git a/src/Orchard.Web/Modules/Orchard.Users/Controllers/AdminController.cs b/src/Orchard.Web/Modules/Orchard.Users/Controllers/AdminController.cs index d6f95b0ca..e85de7d77 100644 --- a/src/Orchard.Web/Modules/Orchard.Users/Controllers/AdminController.cs +++ b/src/Orchard.Web/Modules/Orchard.Users/Controllers/AdminController.cs @@ -1,7 +1,7 @@ using System.Linq; using System.Web.Mvc; -using Orchard.Localization; using Orchard.ContentManagement; +using Orchard.Localization; using Orchard.Security; using Orchard.UI.Notify; using Orchard.Users.Drivers; @@ -147,6 +147,7 @@ namespace Orchard.Users.Controllers { bool IUpdateModel.TryUpdateModel(TModel model, string prefix, string[] includeProperties, string[] excludeProperties) { return TryUpdateModel(model, prefix, includeProperties, excludeProperties); } + public void AddModelError(string key, LocalizedString errorMessage) { ModelState.AddModelError(key, errorMessage.ToString()); } diff --git a/src/Orchard.Web/Modules/Orchard.Users/Services/MembershipService.cs b/src/Orchard.Web/Modules/Orchard.Users/Services/MembershipService.cs index a7fb519fb..3d44d1169 100644 --- a/src/Orchard.Web/Modules/Orchard.Users/Services/MembershipService.cs +++ b/src/Orchard.Web/Modules/Orchard.Users/Services/MembershipService.cs @@ -44,6 +44,9 @@ namespace Orchard.Users.Services { } public IUser GetUser(string username) { + if(username == null) { + throw new ArgumentNullException("username"); + } var userRecord = _userRepository.Get(x => x.NormalizedUserName == username.ToLower()); if (userRecord == null) { return null; @@ -51,8 +54,10 @@ namespace Orchard.Users.Services { return _contentManager.Get(userRecord.Id); } - public IUser ValidateUser(string username, string password) { - var userRecord = _userRepository.Get(x => x.NormalizedUserName == username.ToLower()); + public IUser ValidateUser(string userNameOrEmail, string password) { + var userRecord = _userRepository.Get(x => x.NormalizedUserName == userNameOrEmail.ToLower()); + if(userRecord == null) + userRecord = _userRepository.Get(x => x.Email == userNameOrEmail.ToLower()); if (userRecord == null || ValidatePassword(userRecord, password) == false) return null; diff --git a/src/Orchard.Web/Modules/Orchard.Users/Views/Account/LogOn.ascx b/src/Orchard.Web/Modules/Orchard.Users/Views/Account/LogOn.ascx index d910ccb98..de5f098cd 100644 --- a/src/Orchard.Web/Modules/Orchard.Users/Views/Account/LogOn.ascx +++ b/src/Orchard.Web/Modules/Orchard.Users/Views/Account/LogOn.ascx @@ -8,9 +8,9 @@ using (Html.BeginFormAntiForgeryPost(Url.Action("LogOn", new {ReturnUrl = Reques
<%=_Encoded("Account Information")%>
- - <%= Html.TextBox("username")%> - <%= Html.ValidationMessage("username")%> + + <%= Html.TextBox("userNameOrEmail")%> + <%= Html.ValidationMessage("userNameOrEmail")%>
diff --git a/src/Orchard/Mvc/Extensions/ModelStateDictionaryExtensions.cs b/src/Orchard/Mvc/Extensions/ModelStateDictionaryExtensions.cs new file mode 100644 index 000000000..e68990229 --- /dev/null +++ b/src/Orchard/Mvc/Extensions/ModelStateDictionaryExtensions.cs @@ -0,0 +1,11 @@ +using System; +using System.Web.Mvc; +using Orchard.Localization; + +namespace Orchard.Mvc.Extensions { + public static class ModelStateDictionaryExtensions { + public static void AddModelError(this ModelStateDictionary modelStateDictionary, string key, LocalizedString errorMessage) { + modelStateDictionary.AddModelError(key, errorMessage.ToString()); + } + } +} diff --git a/src/Orchard/Orchard.Framework.csproj b/src/Orchard/Orchard.Framework.csproj index 2de8742aa..9beecc61b 100644 --- a/src/Orchard/Orchard.Framework.csproj +++ b/src/Orchard/Orchard.Framework.csproj @@ -195,6 +195,7 @@ + diff --git a/src/Orchard/Security/IMembershipService.cs b/src/Orchard/Security/IMembershipService.cs index 44578cac1..0821fcdb1 100644 --- a/src/Orchard/Security/IMembershipService.cs +++ b/src/Orchard/Security/IMembershipService.cs @@ -5,7 +5,7 @@ IUser CreateUser(CreateUserParams createUserParams); IUser GetUser(string username); - IUser ValidateUser(string username, string password); + IUser ValidateUser(string userNameOrEmail, string password); void SetPassword(IUser user, string password); } }