From dc32138006d5d6c7bdb64e5ef96bed7a6c844614 Mon Sep 17 00:00:00 2001 From: AndyButland Date: Tue, 12 Aug 2014 08:15:43 +0100 Subject: [PATCH 1/9] Set up database level paging for retrieving child content for list view --- src/Umbraco.Core/Persistence/PetaPoco.cs | 5 ++ .../Repositories/ContentRepository.cs | 67 ++++++++++++++++++- .../Interfaces/IContentRepository.cs | 13 ++++ src/Umbraco.Core/Services/ContentService.cs | 24 +++++++ src/Umbraco.Core/Services/IContentService.cs | 14 ++++ src/Umbraco.Web/Editors/ContentController.cs | 50 +++++--------- 6 files changed, 137 insertions(+), 36 deletions(-) diff --git a/src/Umbraco.Core/Persistence/PetaPoco.cs b/src/Umbraco.Core/Persistence/PetaPoco.cs index d8507ac681..41abe7640e 100644 --- a/src/Umbraco.Core/Persistence/PetaPoco.cs +++ b/src/Umbraco.Core/Persistence/PetaPoco.cs @@ -2309,6 +2309,11 @@ namespace Umbraco.Core.Persistence return Append(new Sql("ORDER BY " + String.Join(", ", (from x in columns select x.ToString()).ToArray()))); } + public Sql OrderByDescending(params object[] columns) + { + return Append(new Sql("ORDER BY " + String.Join(", ", (from x in columns select x.ToString() + " DESC").ToArray()))); + } + public Sql Select(params object[] columns) { return Append(new Sql("SELECT " + String.Join(", ", (from x in columns select x.ToString()).ToArray()))); diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs index c1633daa75..ed1a87b2de 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs @@ -3,8 +3,10 @@ using System.Collections.Generic; using System.Data; using System.Globalization; using System.Linq; +using System.Linq.Expressions; using System.Xml.Linq; using Umbraco.Core.Configuration; +using Umbraco.Core.Dynamics; using Umbraco.Core.IO; using Umbraco.Core.Models; using Umbraco.Core.Models.EntityBase; @@ -636,7 +638,70 @@ namespace Umbraco.Core.Persistence.Repositories _contentPreviewRepository.AddOrUpdate(new ContentPreviewEntity(previewExists, content, xml)); } - + + /// + /// Gets paged content results + /// + /// Query to excute + /// Page number + /// Page size + /// Total records query would return without paging + /// Field to order by + /// Direction to order by + /// An Enumerable list of objects + public IEnumerable GetPagedResultsByQuery(IQuery query, int pageNumber, int pageSize, out int totalRecords, + string orderBy, Direction orderDirection) + { + // Get base query + var sqlClause = GetBaseQuery(false); + var translator = new SqlTranslator(sqlClause, query); + var sql = translator.Translate() + .Where(x => x.Newest); + + // Apply order by according to parameters + var orderByParams = new[] { orderBy }; + if (orderDirection == Direction.Ascending) + { + sql = sql.OrderBy(orderByParams); + } + else + { + sql = sql.OrderByDescending(orderByParams); + } + + // Note we can't do multi-page for several DTOs like we can multi-fetch and are doing in PerformGetByQuery, + // but actually given we are doing a Get on each one (again as in PerformGetByQuery), we only need the node Id + IEnumerable result; + var pagedResult = Database.Page(pageNumber, pageSize, sql.SQL.Replace("SELECT *", "SELECT cmsDocument.nodeId")); + totalRecords = Convert.ToInt32(pagedResult.TotalItems); + if (totalRecords > 0) + { + // Parse out node Ids and load content (we need the cast here in order to be able to call the IQueryable extension + // methods OrderBy or OrderByDescending) + var content = GetAll(pagedResult.Items + .DistinctBy(x => x.NodeId) + .Select(x => x.NodeId).ToArray()) + .Cast() + .AsQueryable(); + + // Now we need to ensure this result is also ordered by the same order by clause + if (orderDirection == Direction.Ascending) + { + result = content.OrderBy(orderBy); + } + else + { + result = content.OrderByDescending(orderBy); + } + } + else + { + result = Enumerable.Empty(); + } + + return result; + } + #endregion /// diff --git a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IContentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IContentRepository.cs index 2fd0a5685e..46bcd7666f 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IContentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IContentRepository.cs @@ -1,8 +1,10 @@ using System; using System.Collections.Generic; +using System.Linq.Expressions; using System.Xml.Linq; using Umbraco.Core.Models; using Umbraco.Core.Models.Membership; +using Umbraco.Core.Persistence.DatabaseModelDefinitions; using Umbraco.Core.Persistence.Querying; namespace Umbraco.Core.Persistence.Repositories @@ -66,5 +68,16 @@ namespace Umbraco.Core.Persistence.Repositories /// void AddOrUpdatePreviewXml(IContent content, Func xml); + /// + /// Gets paged content results + /// + /// Query to excute + /// Page number + /// Page size + /// Total records query would return without paging + /// Field to order by + /// Direction to order by + /// An Enumerable list of objects + IEnumerable GetPagedResultsByQuery(IQuery query, int pageNumber, int pageSize, out int totalRecords, string orderBy, Direction orderDirection); } } \ No newline at end of file diff --git a/src/Umbraco.Core/Services/ContentService.cs b/src/Umbraco.Core/Services/ContentService.cs index 311e68a482..27fd5c1f3d 100644 --- a/src/Umbraco.Core/Services/ContentService.cs +++ b/src/Umbraco.Core/Services/ContentService.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Generic; using System.Globalization; using System.Linq; +using System.Linq.Expressions; using System.Threading; using System.Xml.Linq; using Umbraco.Core.Auditing; @@ -13,6 +14,7 @@ using Umbraco.Core.Models.Membership; using Umbraco.Core.Models.Rdbms; using Umbraco.Core.Persistence; using Umbraco.Core.Persistence.Caching; +using Umbraco.Core.Persistence.DatabaseModelDefinitions; using Umbraco.Core.Persistence.Querying; using Umbraco.Core.Persistence.Repositories; using Umbraco.Core.Persistence.SqlSyntax; @@ -473,6 +475,28 @@ namespace Umbraco.Core.Services } } + /// + /// Gets a collection of objects by Parent Id + /// + /// Id of the Parent to retrieve Children from + /// Page number + /// Page size + /// Total records query would return without paging + /// Field to order by + /// Direction to order by + /// An Enumerable list of objects + public IEnumerable GetPagedChildren(int id, int pageNumber, int pageSize, out int totalChildren, + string orderBy, Direction orderDirection) + { + using (var repository = _repositoryFactory.CreateContentRepository(_uowProvider.GetUnitOfWork())) + { + var query = Query.Builder.Where(x => x.ParentId == id); + var contents = repository.GetPagedResultsByQuery(query, pageNumber, pageSize, out totalChildren, orderBy, orderDirection); + + return contents; + } + } + /// /// Gets a collection of objects by its name or partial name /// diff --git a/src/Umbraco.Core/Services/IContentService.cs b/src/Umbraco.Core/Services/IContentService.cs index 76964e8e9a..efb045f6f6 100644 --- a/src/Umbraco.Core/Services/IContentService.cs +++ b/src/Umbraco.Core/Services/IContentService.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Generic; using Umbraco.Core.Models; using Umbraco.Core.Models.Membership; +using Umbraco.Core.Persistence.DatabaseModelDefinitions; using Umbraco.Core.Publishing; namespace Umbraco.Core.Services @@ -108,6 +109,19 @@ namespace Umbraco.Core.Services /// An Enumerable list of objects IEnumerable GetChildren(int id); + /// + /// Gets a collection of objects by Parent Id + /// + /// Id of the Parent to retrieve Children from + /// Page number + /// Page size + /// Total records query would return without paging + /// Field to order by + /// Direction to order by + /// An Enumerable list of objects + IEnumerable GetPagedChildren(int id, int pageNumber, int pageSize, out int totalChildren, + string orderBy, Direction orderDirection); + /// /// Gets a collection of an objects versions by its Id /// diff --git a/src/Umbraco.Web/Editors/ContentController.cs b/src/Umbraco.Web/Editors/ContentController.cs index d2a023ee98..49a0f12e1f 100644 --- a/src/Umbraco.Web/Editors/ContentController.cs +++ b/src/Umbraco.Web/Editors/ContentController.cs @@ -157,47 +157,27 @@ namespace Umbraco.Web.Editors Direction orderDirection = Direction.Ascending, string filter = "") { - //TODO: This will be horribly inefficient for paging! This is because our datasource/repository - // doesn't support paging at the SQL level... and it'll be pretty interesting to try to make that work. + //TODO: move this filter to repo level as needs to be included in paging + //if (!string.IsNullOrEmpty(filter)) + //{ + // filter = filter.ToLower(); + // result = result.Where(x => x.Name.InvariantContains(filter)); + //} - //PP: could we in 7.0.1+ migrate this to the internal examine index instead of using the content service? - - var children = Services.ContentService.GetChildren(id).ToArray(); - var totalChildren = children.Length; + int totalChildren; + var children = Services.ContentService.GetPagedChildren(id, pageNumber, pageSize, out totalChildren, orderBy, orderDirection).ToArray(); if (totalChildren == 0) + { return new PagedResult>(0, 0, 0); - - var result = children - .Select(Mapper.Map>) - .AsQueryable(); - - //TODO: This is a rudimentry filter - should use the logic found in the EntityService filter (dynamic linq) instead - if (!string.IsNullOrEmpty(filter)) - { - filter = filter.ToLower(); - result = result.Where(x => x.Name.InvariantContains(filter)); } - var orderedResult = orderDirection == Direction.Ascending - ? result.OrderBy(orderBy) - : result.OrderByDescending(orderBy); - - var pagedResult = new PagedResult>( - totalChildren, - pageNumber, - pageSize); - - if (pageNumber > 0 && pageSize > 0) - { - pagedResult.Items = orderedResult - .Skip(pagedResult.SkipSize) - .Take(pageSize); - } - else - { - pagedResult.Items = orderedResult; - } + var pagedResult = new PagedResult>( + totalChildren, + pageNumber, + pageSize); + pagedResult.Items = children + .Select(Mapper.Map>); return pagedResult; } From a71bfaa14cb079d0d8664403af1610cf5b3f6d03 Mon Sep 17 00:00:00 2001 From: AndyButland Date: Tue, 12 Aug 2014 23:32:13 +0100 Subject: [PATCH 2/9] Amends to generated SQL pre-paging to ensure only the latest content versions are returned --- .../Persistence/Repositories/ContentRepository.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs index ed1a87b2de..c4c01391b6 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs @@ -671,8 +671,15 @@ namespace Umbraco.Core.Persistence.Repositories // Note we can't do multi-page for several DTOs like we can multi-fetch and are doing in PerformGetByQuery, // but actually given we are doing a Get on each one (again as in PerformGetByQuery), we only need the node Id + // So we'll modify the SQL + var modifiedSQL = sql.SQL.Replace("SELECT *", "SELECT cmsDocument.nodeId"); + + // This I don't follow, but the "Newest" where clause added above doesn't get into the generated SQL + // So we'll add it. + modifiedSQL = modifiedSQL.Replace("WHERE ", "WHERE Newest = 1 AND "); + IEnumerable result; - var pagedResult = Database.Page(pageNumber, pageSize, sql.SQL.Replace("SELECT *", "SELECT cmsDocument.nodeId")); + var pagedResult = Database.Page(pageNumber, pageSize, modifiedSQL); totalRecords = Convert.ToInt32(pagedResult.TotalItems); if (totalRecords > 0) { From 817d5aab0d13b816d7b0c806d5785b04c6fa6602 Mon Sep 17 00:00:00 2001 From: AndyButland Date: Wed, 13 Aug 2014 20:02:51 +0100 Subject: [PATCH 3/9] Moved list view filter to repository, so can be applied with a database query --- .../Repositories/ContentRepository.cs | 23 ++++++++++++++----- .../Interfaces/IContentRepository.cs | 4 +++- src/Umbraco.Core/Services/ContentService.cs | 7 +++--- src/Umbraco.Core/Services/IContentService.cs | 3 ++- src/Umbraco.Web/Editors/ContentController.cs | 14 ++--------- 5 files changed, 28 insertions(+), 23 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs index c4c01391b6..c5454b133f 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs @@ -648,9 +648,10 @@ namespace Umbraco.Core.Persistence.Repositories /// Total records query would return without paging /// Field to order by /// Direction to order by + /// Search text filter /// An Enumerable list of objects public IEnumerable GetPagedResultsByQuery(IQuery query, int pageNumber, int pageSize, out int totalRecords, - string orderBy, Direction orderDirection) + string orderBy, Direction orderDirection, string filter = "") { // Get base query var sqlClause = GetBaseQuery(false); @@ -658,7 +659,7 @@ namespace Umbraco.Core.Persistence.Repositories var sql = translator.Translate() .Where(x => x.Newest); - // Apply order by according to parameters + // Apply order according to parameters var orderByParams = new[] { orderBy }; if (orderDirection == Direction.Ascending) { @@ -670,14 +671,24 @@ namespace Umbraco.Core.Persistence.Repositories } // Note we can't do multi-page for several DTOs like we can multi-fetch and are doing in PerformGetByQuery, - // but actually given we are doing a Get on each one (again as in PerformGetByQuery), we only need the node Id - // So we'll modify the SQL + // but actually given we are doing a Get on each one (again as in PerformGetByQuery), we only need the node Id. + // So we'll modify the SQL. var modifiedSQL = sql.SQL.Replace("SELECT *", "SELECT cmsDocument.nodeId"); - // This I don't follow, but the "Newest" where clause added above doesn't get into the generated SQL - // So we'll add it. + // HACK: the .Where(x => x.Newest) clause above is also being used in PerformGetQuery, so included here, + // but it doesn't look to do anything as the clause isn't added to the the generated SQL. + // So we'll add it here. modifiedSQL = modifiedSQL.Replace("WHERE ", "WHERE Newest = 1 AND "); + // HACK: Apply filter. Again, can't get expression based Where filter to be added to the generated SQL, + // so working with the raw string. Potential SQL injection here so, although escaped, should be modified. + if (!string.IsNullOrEmpty(filter)) + { + modifiedSQL = modifiedSQL.Replace("WHERE ", + string.Format("WHERE cmsDocument.text LIKE '%{0}%' AND ", filter.Replace("'", "''"))); + } + + // Get page of results and total count IEnumerable result; var pagedResult = Database.Page(pageNumber, pageSize, modifiedSQL); totalRecords = Convert.ToInt32(pagedResult.TotalItems); diff --git a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IContentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IContentRepository.cs index 46bcd7666f..ba99e50641 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IContentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IContentRepository.cs @@ -77,7 +77,9 @@ namespace Umbraco.Core.Persistence.Repositories /// Total records query would return without paging /// Field to order by /// Direction to order by + /// Search text filter /// An Enumerable list of objects - IEnumerable GetPagedResultsByQuery(IQuery query, int pageNumber, int pageSize, out int totalRecords, string orderBy, Direction orderDirection); + IEnumerable GetPagedResultsByQuery(IQuery query, int pageNumber, int pageSize, out int totalRecords, + string orderBy, Direction orderDirection, string filter = ""); } } \ No newline at end of file diff --git a/src/Umbraco.Core/Services/ContentService.cs b/src/Umbraco.Core/Services/ContentService.cs index 27fd5c1f3d..9ac8ba4162 100644 --- a/src/Umbraco.Core/Services/ContentService.cs +++ b/src/Umbraco.Core/Services/ContentService.cs @@ -484,14 +484,15 @@ namespace Umbraco.Core.Services /// Total records query would return without paging /// Field to order by /// Direction to order by + /// Search text filter /// An Enumerable list of objects - public IEnumerable GetPagedChildren(int id, int pageNumber, int pageSize, out int totalChildren, - string orderBy, Direction orderDirection) + public IEnumerable GetPagedChildren(int id, int pageNumber, int pageSize, out int totalChildren, + string orderBy, Direction orderDirection, string filter = "") { using (var repository = _repositoryFactory.CreateContentRepository(_uowProvider.GetUnitOfWork())) { var query = Query.Builder.Where(x => x.ParentId == id); - var contents = repository.GetPagedResultsByQuery(query, pageNumber, pageSize, out totalChildren, orderBy, orderDirection); + var contents = repository.GetPagedResultsByQuery(query, pageNumber, pageSize, out totalChildren, orderBy, orderDirection, filter); return contents; } diff --git a/src/Umbraco.Core/Services/IContentService.cs b/src/Umbraco.Core/Services/IContentService.cs index efb045f6f6..a2e44578a8 100644 --- a/src/Umbraco.Core/Services/IContentService.cs +++ b/src/Umbraco.Core/Services/IContentService.cs @@ -118,9 +118,10 @@ namespace Umbraco.Core.Services /// Total records query would return without paging /// Field to order by /// Direction to order by + /// Search text filter /// An Enumerable list of objects IEnumerable GetPagedChildren(int id, int pageNumber, int pageSize, out int totalChildren, - string orderBy, Direction orderDirection); + string orderBy, Direction orderDirection, string filter = ""); /// /// Gets a collection of an objects versions by its Id diff --git a/src/Umbraco.Web/Editors/ContentController.cs b/src/Umbraco.Web/Editors/ContentController.cs index 49a0f12e1f..b9cb9d07a2 100644 --- a/src/Umbraco.Web/Editors/ContentController.cs +++ b/src/Umbraco.Web/Editors/ContentController.cs @@ -157,25 +157,15 @@ namespace Umbraco.Web.Editors Direction orderDirection = Direction.Ascending, string filter = "") { - //TODO: move this filter to repo level as needs to be included in paging - //if (!string.IsNullOrEmpty(filter)) - //{ - // filter = filter.ToLower(); - // result = result.Where(x => x.Name.InvariantContains(filter)); - //} - int totalChildren; - var children = Services.ContentService.GetPagedChildren(id, pageNumber, pageSize, out totalChildren, orderBy, orderDirection).ToArray(); + var children = Services.ContentService.GetPagedChildren(id, pageNumber, pageSize, out totalChildren, orderBy, orderDirection, filter).ToArray(); if (totalChildren == 0) { return new PagedResult>(0, 0, 0); } - var pagedResult = new PagedResult>( - totalChildren, - pageNumber, - pageSize); + var pagedResult = new PagedResult>(totalChildren, pageNumber, pageSize); pagedResult.Items = children .Select(Mapper.Map>); From 096e10aad4a418b9fa09f5736d519886e82b2f8b Mon Sep 17 00:00:00 2001 From: AndyButland Date: Wed, 13 Aug 2014 20:06:55 +0100 Subject: [PATCH 4/9] Moved list view no results message into table body as otherwise if you apply a filter that returns no results, you can't clear it again --- .../propertyeditors/listview/listview.html | 28 ++++++++++++------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/listview/listview.html b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/listview/listview.html index 57b7eff7c9..9155a63419 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/listview/listview.html +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/listview/listview.html @@ -41,11 +41,7 @@ -

- There are no items show in the list. -

- - +
- + + + + + + + + + - + - From f6ec4798a60267b32ae645e1fbf721bb649cf015 Mon Sep 17 00:00:00 2001 From: AndyButland Date: Wed, 13 Aug 2014 22:11:41 +0100 Subject: [PATCH 5/9] Prevented list view filter box from setting dirty flag on form. Without this you get the save changes warning after using the filter, but there's nothing to save. --- .../directives/utill/nodirtycheck.directive.js | 18 ++++++++++++++++++ .../validation/valformmanager.directive.js | 1 + .../propertyeditors/listview/listview.html | 2 +- 3 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 src/Umbraco.Web.UI.Client/src/common/directives/utill/nodirtycheck.directive.js diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/utill/nodirtycheck.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/utill/nodirtycheck.directive.js new file mode 100644 index 0000000000..74c007dfbc --- /dev/null +++ b/src/Umbraco.Web.UI.Client/src/common/directives/utill/nodirtycheck.directive.js @@ -0,0 +1,18 @@ +/** +* @ngdoc directive +* @name umbraco.directives.directive:noDirtyCheck +* @restrict A +* @description Can be attached to form inputs to prevent them from setting the form as dirty (http://stackoverflow.com/questions/17089090/prevent-input-from-setting-form-dirty-angularjs) +**/ +function noDirtyCheck() { + return { + restrict: 'A', + require: 'ngModel', + link: function (scope, elm, attrs, ctrl) { + elm.focus(function () { + ctrl.$pristine = false; + }); + } + }; +} +angular.module('umbraco.directives').directive("noDirtyCheck", noDirtyCheck); \ No newline at end of file diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/validation/valformmanager.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/validation/valformmanager.directive.js index 2e5aa52e80..0ad344bd1d 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/validation/valformmanager.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/validation/valformmanager.directive.js @@ -64,6 +64,7 @@ function valFormManager(serverValidationManager, $rootScope, $log, $timeout, not return; } + console.log("I am dirty"); var path = nextLocation.split("#")[1]; if (path) { if (path.indexOf("%253") || path.indexOf("%252")) { diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/listview/listview.html b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/listview/listview.html index 9155a63419..d2edfa2f10 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/listview/listview.html +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/listview/listview.html @@ -58,7 +58,7 @@ From 8ea5b6fb46d1e15ad89b3b936ec8862aab55bcf8 Mon Sep 17 00:00:00 2001 From: AndyButland Date: Thu, 14 Aug 2014 00:16:37 +0200 Subject: [PATCH 6/9] Unit tests for content repository paged queries; amends to fix ordering issues --- .../Repositories/ContentRepository.cs | 54 ++++++++++-- .../Repositories/ContentRepositoryTest.cs | 85 +++++++++++++++++++ 2 files changed, 130 insertions(+), 9 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs index c5454b133f..85e33d8c7f 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs @@ -660,14 +660,17 @@ namespace Umbraco.Core.Persistence.Repositories .Where(x => x.Newest); // Apply order according to parameters - var orderByParams = new[] { orderBy }; - if (orderDirection == Direction.Ascending) + if (!string.IsNullOrEmpty(orderBy)) { - sql = sql.OrderBy(orderByParams); - } - else - { - sql = sql.OrderByDescending(orderByParams); + var orderByParams = new[] { GetDatabaseFieldNameForOrderBy(orderBy) }; + if (orderDirection == Direction.Ascending) + { + sql = sql.OrderBy(orderByParams); + } + else + { + sql = sql.OrderByDescending(orderByParams); + } } // Note we can't do multi-page for several DTOs like we can multi-fetch and are doing in PerformGetByQuery, @@ -703,13 +706,14 @@ namespace Umbraco.Core.Persistence.Repositories .AsQueryable(); // Now we need to ensure this result is also ordered by the same order by clause + var orderByProperty = GetIContentPropertyNameForOrderBy(orderBy); if (orderDirection == Direction.Ascending) { - result = content.OrderBy(orderBy); + result = content.OrderBy(orderByProperty); } else { - result = content.OrderByDescending(orderBy); + result = content.OrderByDescending(orderByProperty); } } else @@ -720,6 +724,38 @@ namespace Umbraco.Core.Persistence.Repositories return result; } + private string GetDatabaseFieldNameForOrderBy(string orderBy) + { + // Translate the passed order by field (which were originally defined for in-memory object sorting + // of ContentItemBasic instances) to the database field names. + switch (orderBy) + { + case "Name": + return "cmsDocument.text"; + case "Owner": + return "umbracoNode.nodeUser"; + case "Updator": + return "cmsDocument.documentUser"; + default: + return orderBy; + } + } + + private string GetIContentPropertyNameForOrderBy(string orderBy) + { + // Translate the passed order by field (which were originally defined for in-memory object sorting + // of ContentItemBasic instances) to the IContent property names. + switch (orderBy) + { + case "Owner": + return "CreatorId"; + case "Updator": + return "WriterId"; + default: + return orderBy; + } + } + #endregion /// diff --git a/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs index e75eabff4f..6903f6d983 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs @@ -15,6 +15,7 @@ using Umbraco.Tests.TestHelpers; using Umbraco.Tests.TestHelpers.Entities; using umbraco.editorControls.tinyMCE3; using umbraco.interfaces; +using Umbraco.Core.Persistence.DatabaseModelDefinitions; namespace Umbraco.Tests.Persistence.Repositories { @@ -331,6 +332,90 @@ namespace Umbraco.Tests.Persistence.Repositories } } + [Test] + public void Can_Perform_GetPagedResultsByQuery_ForFirstPage_On_ContentRepository() + { + // Arrange + var provider = new PetaPocoUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); + ContentTypeRepository contentTypeRepository; + using (var repository = CreateRepository(unitOfWork, out contentTypeRepository)) + { + // Act + var query = Query.Builder.Where(x => x.Level == 2); + int totalRecords; + var result = repository.GetPagedResultsByQuery(query, 1, 1, out totalRecords, "Name", Direction.Ascending); + + // Assert + Assert.That(totalRecords, Is.GreaterThanOrEqualTo(2)); + Assert.That(result.Count(), Is.EqualTo(1)); + Assert.That(result.First().Name, Is.EqualTo("Text Page 1")); + } + } + + [Test] + public void Can_Perform_GetPagedResultsByQuery_ForSecondPage_On_ContentRepository() + { + // Arrange + var provider = new PetaPocoUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); + ContentTypeRepository contentTypeRepository; + using (var repository = CreateRepository(unitOfWork, out contentTypeRepository)) + { + // Act + var query = Query.Builder.Where(x => x.Level == 2); + int totalRecords; + var result = repository.GetPagedResultsByQuery(query, 2, 1, out totalRecords, "Name", Direction.Ascending); + + // Assert + Assert.That(totalRecords, Is.GreaterThanOrEqualTo(2)); + Assert.That(result.Count(), Is.EqualTo(1)); + Assert.That(result.First().Name, Is.EqualTo("Text Page 2")); + } + } + + [Test] + public void Can_Perform_GetPagedResultsByQuery_WithDescendingOrder_On_ContentRepository() + { + // Arrange + var provider = new PetaPocoUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); + ContentTypeRepository contentTypeRepository; + using (var repository = CreateRepository(unitOfWork, out contentTypeRepository)) + { + // Act + var query = Query.Builder.Where(x => x.Level == 2); + int totalRecords; + var result = repository.GetPagedResultsByQuery(query, 1, 1, out totalRecords, "Name", Direction.Descending); + + // Assert + Assert.That(totalRecords, Is.GreaterThanOrEqualTo(2)); + Assert.That(result.Count(), Is.EqualTo(1)); + Assert.That(result.First().Name, Is.EqualTo("Text Page 2")); + } + } + + [Test] + public void Can_Perform_GetPagedResultsByQuery_WithFilter_On_ContentRepository() + { + // Arrange + var provider = new PetaPocoUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); + ContentTypeRepository contentTypeRepository; + using (var repository = CreateRepository(unitOfWork, out contentTypeRepository)) + { + // Act + var query = Query.Builder.Where(x => x.Level == 2); + int totalRecords; + var result = repository.GetPagedResultsByQuery(query, 1, 1, out totalRecords, "Name", Direction.Ascending, "Page 2"); + + // Assert + Assert.That(totalRecords, Is.EqualTo(1)); + Assert.That(result.Count(), Is.EqualTo(1)); + Assert.That(result.First().Name, Is.EqualTo("Text Page 2")); + } + } + [Test] public void Can_Perform_GetAll_By_Param_Ids_On_ContentRepository() { From ca2dd3eb470a46cca4d1cc63d637c34ea3a30003 Mon Sep 17 00:00:00 2001 From: AndyButland Date: Thu, 14 Aug 2014 08:42:36 +0200 Subject: [PATCH 7/9] Fixed issue with list view filter; further unit tests for list view paging --- .../Repositories/ContentRepository.cs | 21 +++------ .../Repositories/ContentRepositoryTest.cs | 46 ++++++++++++++++++- 2 files changed, 51 insertions(+), 16 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs index 85e33d8c7f..42155d0baf 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs @@ -659,6 +659,12 @@ namespace Umbraco.Core.Persistence.Repositories var sql = translator.Translate() .Where(x => x.Newest); + // Apply filter + if (!string.IsNullOrEmpty(filter)) + { + sql = sql.Where("cmsDocument.text LIKE @0", "%" + filter + "%"); + } + // Apply order according to parameters if (!string.IsNullOrEmpty(orderBy)) { @@ -678,22 +684,9 @@ namespace Umbraco.Core.Persistence.Repositories // So we'll modify the SQL. var modifiedSQL = sql.SQL.Replace("SELECT *", "SELECT cmsDocument.nodeId"); - // HACK: the .Where(x => x.Newest) clause above is also being used in PerformGetQuery, so included here, - // but it doesn't look to do anything as the clause isn't added to the the generated SQL. - // So we'll add it here. - modifiedSQL = modifiedSQL.Replace("WHERE ", "WHERE Newest = 1 AND "); - - // HACK: Apply filter. Again, can't get expression based Where filter to be added to the generated SQL, - // so working with the raw string. Potential SQL injection here so, although escaped, should be modified. - if (!string.IsNullOrEmpty(filter)) - { - modifiedSQL = modifiedSQL.Replace("WHERE ", - string.Format("WHERE cmsDocument.text LIKE '%{0}%' AND ", filter.Replace("'", "''"))); - } - // Get page of results and total count IEnumerable result; - var pagedResult = Database.Page(pageNumber, pageSize, modifiedSQL); + var pagedResult = Database.Page(pageNumber, pageSize, modifiedSQL, sql.Arguments); totalRecords = Convert.ToInt32(pagedResult.TotalItems); if (totalRecords > 0) { diff --git a/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs index 6903f6d983..26510f3b5d 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs @@ -374,6 +374,27 @@ namespace Umbraco.Tests.Persistence.Repositories } } + [Test] + public void Can_Perform_GetPagedResultsByQuery_WithSinglePage_On_ContentRepository() + { + // Arrange + var provider = new PetaPocoUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); + ContentTypeRepository contentTypeRepository; + using (var repository = CreateRepository(unitOfWork, out contentTypeRepository)) + { + // Act + var query = Query.Builder.Where(x => x.Level == 2); + int totalRecords; + var result = repository.GetPagedResultsByQuery(query, 1, 2, out totalRecords, "Name", Direction.Ascending); + + // Assert + Assert.That(totalRecords, Is.GreaterThanOrEqualTo(2)); + Assert.That(result.Count(), Is.EqualTo(2)); + Assert.That(result.First().Name, Is.EqualTo("Text Page 1")); + } + } + [Test] public void Can_Perform_GetPagedResultsByQuery_WithDescendingOrder_On_ContentRepository() { @@ -396,7 +417,7 @@ namespace Umbraco.Tests.Persistence.Repositories } [Test] - public void Can_Perform_GetPagedResultsByQuery_WithFilter_On_ContentRepository() + public void Can_Perform_GetPagedResultsByQuery_WithFilterMatchingSome_On_ContentRepository() { // Arrange var provider = new PetaPocoUnitOfWorkProvider(); @@ -415,7 +436,28 @@ namespace Umbraco.Tests.Persistence.Repositories Assert.That(result.First().Name, Is.EqualTo("Text Page 2")); } } - + + [Test] + public void Can_Perform_GetPagedResultsByQuery_WithFilterMatchingAll_On_ContentRepository() + { + // Arrange + var provider = new PetaPocoUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); + ContentTypeRepository contentTypeRepository; + using (var repository = CreateRepository(unitOfWork, out contentTypeRepository)) + { + // Act + var query = Query.Builder.Where(x => x.Level == 2); + int totalRecords; + var result = repository.GetPagedResultsByQuery(query, 1, 1, out totalRecords, "Name", Direction.Ascending, "Page"); + + // Assert + Assert.That(totalRecords, Is.EqualTo(2)); + Assert.That(result.Count(), Is.EqualTo(1)); + Assert.That(result.First().Name, Is.EqualTo("Text Page 1")); + } + } + [Test] public void Can_Perform_GetAll_By_Param_Ids_On_ContentRepository() { From beda0968ae825a07cb5fd890ff1f27c386031162 Mon Sep 17 00:00:00 2001 From: AndyButland Date: Thu, 14 Aug 2014 18:07:21 +0200 Subject: [PATCH 8/9] Removed debug line --- .../src/common/directives/validation/valformmanager.directive.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/validation/valformmanager.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/validation/valformmanager.directive.js index 0ad344bd1d..2e5aa52e80 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/validation/valformmanager.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/validation/valformmanager.directive.js @@ -64,7 +64,6 @@ function valFormManager(serverValidationManager, $rootScope, $log, $timeout, not return; } - console.log("I am dirty"); var path = nextLocation.split("#")[1]; if (path) { if (path.indexOf("%253") || path.indexOf("%252")) { From 53097a9b848cd2ac78af1ae9c2bf8f60d962cabd Mon Sep 17 00:00:00 2001 From: AndyButland Date: Thu, 14 Aug 2014 18:16:49 +0200 Subject: [PATCH 9/9] Moved added method from PetaPoco file to an extension method --- src/Umbraco.Core/Persistence/PetaPoco.cs | 5 ----- src/Umbraco.Core/Persistence/PetaPocoSqlExtensions.cs | 6 ++++++ 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/Umbraco.Core/Persistence/PetaPoco.cs b/src/Umbraco.Core/Persistence/PetaPoco.cs index 41abe7640e..d8507ac681 100644 --- a/src/Umbraco.Core/Persistence/PetaPoco.cs +++ b/src/Umbraco.Core/Persistence/PetaPoco.cs @@ -2309,11 +2309,6 @@ namespace Umbraco.Core.Persistence return Append(new Sql("ORDER BY " + String.Join(", ", (from x in columns select x.ToString()).ToArray()))); } - public Sql OrderByDescending(params object[] columns) - { - return Append(new Sql("ORDER BY " + String.Join(", ", (from x in columns select x.ToString() + " DESC").ToArray()))); - } - public Sql Select(params object[] columns) { return Append(new Sql("SELECT " + String.Join(", ", (from x in columns select x.ToString()).ToArray()))); diff --git a/src/Umbraco.Core/Persistence/PetaPocoSqlExtensions.cs b/src/Umbraco.Core/Persistence/PetaPocoSqlExtensions.cs index 4e1a3de28f..c606c24a59 100644 --- a/src/Umbraco.Core/Persistence/PetaPocoSqlExtensions.cs +++ b/src/Umbraco.Core/Persistence/PetaPocoSqlExtensions.cs @@ -1,4 +1,5 @@ using System; +using System.Linq; using System.Linq.Expressions; using System.Reflection; using Umbraco.Core.Persistence.Querying; @@ -125,5 +126,10 @@ namespace Umbraco.Core.Persistence SqlSyntaxContext.SqlSyntaxProvider.GetQuotedColumnName(rightColumnName)); return sql.On(onClause); } + + public static Sql OrderByDescending(this Sql sql, params object[] columns) + { + return sql.Append(new Sql("ORDER BY " + String.Join(", ", (from x in columns select x.ToString() + " DESC").ToArray()))); + } } } \ No newline at end of file
@@ -68,20 +64,32 @@
+

There are no items show in the list.

+
- - {{result.name}}{{result.updateDate|date:'medium'}} + {{result.name}} + + {{result.updateDate|date:'medium'}} {{result.owner.name}} + + {{result.owner.name}}
- +