Re: [Freeipa-devel] [PATCH] 202 Password policy paging with proper sorting

2012-08-29 Thread Petr Vobornik

On 08/28/2012 04:50 PM, Endi Sukma Dewata wrote:

ACK.


Pushed to master, ipa-3-0.



Some comments below.

On 8/27/2012 10:51 AM, Petr Vobornik wrote:

This patch adds option to disable sorting when paging. It allowed to
enable paging in password policy with order of items untouched (they are
sorted on server side by priority).


Is the sorting we see in the UI and CLI identical to the actual sorting
used to resolve the policy? I notice that they are sorted
lexicographically instead of numerically.


I hope not. New ticket about the sorting: 
https://fedorahosted.org/freeipa/ticket/3039





Also fixing issue when paging is disabled and command summary = null. It
displayed 'null' in facet footer.

https://fedorahosted.org/freeipa/ticket/2677

Note: personally I would left paging disabled in password policy page. I
don't think it is beneficial. It slows things down and there would be
hardly more than 100 policies.


This should be confirmed by someone more familiar with actual
deployments. I'd have no objections.


--
Petr Vobornik


___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 202 Password policy paging with proper sorting

2012-08-28 Thread Endi Sukma Dewata

ACK.

Some comments below.

On 8/27/2012 10:51 AM, Petr Vobornik wrote:

This patch adds option to disable sorting when paging. It allowed to
enable paging in password policy with order of items untouched (they are
sorted on server side by priority).


Is the sorting we see in the UI and CLI identical to the actual sorting 
used to resolve the policy? I notice that they are sorted 
lexicographically instead of numerically.



Also fixing issue when paging is disabled and command summary = null. It
displayed 'null' in facet footer.

https://fedorahosted.org/freeipa/ticket/2677

Note: personally I would left paging disabled in password policy page. I
don't think it is beneficial. It slows things down and there would be
hardly more than 100 policies.


This should be confirmed by someone more familiar with actual 
deployments. I'd have no objections.


--
Endi S. Dewata

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


[Freeipa-devel] [PATCH] 202 Password policy paging with proper sorting

2012-08-27 Thread Petr Vobornik
This patch adds option to disable sorting when paging. It allowed to 
enable paging in password policy with order of items untouched (they are 
sorted on server side by priority).


Also fixing issue when paging is disabled and command summary = null. It 
displayed 'null' in facet footer.


https://fedorahosted.org/freeipa/ticket/2677

Note: personally I would left paging disabled in password policy page. I 
don't think it is beneficial. It slows things down and there would be 
hardly more than 100 policies.

--
Petr Vobornik

From 2d13697f5d573ea9b8fe4adbcd3f4db7ff31cb3a Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Mon, 27 Aug 2012 17:41:28 +0200
Subject: [PATCH] Password policy paging with proper sorting

This patch adds option to disable sorting when paging. It allowed to enable paging in password policy with order of items untouched (they are sorted on server side by priority).

Also fixing issue when paging is disabled and command summary = null. It displayed 'null' in facet footer.

https://fedorahosted.org/freeipa/ticket/2677
---
 install/ui/facet.js  |7 +--
 install/ui/policy.js |2 +-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/install/ui/facet.js b/install/ui/facet.js
index d5b21f323513fb9567412135dcafb6d7b8b20815..0de08d3f7a031a3ccce326a88e1b3fbe6d8ddb65 100644
--- a/install/ui/facet.js
+++ b/install/ui/facet.js
@@ -740,6 +740,7 @@ IPA.table_facet = function(spec, no_init) {
 that.pagination = spec.pagination === undefined ? true : spec.pagination;
 that.search_all_entries = spec.search_all_entries;
 that.search_all_attributes = spec.search_all_attributes;
+that.sort_enabled = spec.sort_enabled === undefined ? true : spec.sort_enabled;
 that.selectable = spec.selectable === undefined ? true : spec.selectable;
 that.select_changed = IPA.observer();
 
@@ -833,7 +834,7 @@ IPA.table_facet = function(spec, no_init) {
 message = message.replace('${counter}', data.result.count);
 that.table.summary.text(message);
 } else {
-that.table.summary.text(data.result.summary);
+that.table.summary.text(data.result.summary || '');
 }
 };
 
@@ -895,7 +896,9 @@ IPA.table_facet = function(spec, no_init) {
 that.table.summary.text(summary);
 
 // sort map based on primary keys
-records_map = records_map.sort();
+if (that.sort_enabled) {
+records_map = records_map.sort();
+}
 
 // trim map leaving the entries visible in the current page only
 records_map = records_map.slice(start-1, end);
diff --git a/install/ui/policy.js b/install/ui/policy.js
index ff932bec4a2219d4199298f7da6d28a5b5eba1f8..c109163e418527e9212c5a546984a7f4929a8fad 100644
--- a/install/ui/policy.js
+++ b/install/ui/policy.js
@@ -33,7 +33,7 @@ IPA.pwpolicy.entity = function(spec) {
 that.entity_init();
 
 that.builder.search_facet({
-pagination: false,
+sort_enabled: false,
 columns:['cn','cospriority']
 }).
 details_facet({
-- 
1.7.10.4

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel