[Freeipa-devel] [PATCH] 001 fixed empty dns record update

2011-07-25 Thread Petr Vobornik
https://fedorahosted.org/freeipa/ticket/1477

Redirection after updating empty DNS Record (which is deleted).
Added hook to details facet for post update operation.

Petr
From aafbaf2464dcc16f552290f72424170c42055b45 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Fri, 22 Jul 2011 13:24:27 +0200
Subject: [PATCH] fixed empty dns record update

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

Redirection after updating empty DNS Record (which is deleted).
Added hook to details facet for post update operation.
---
 install/ui/details.js  |8 +++-
 install/ui/dns.js  |   34 +-
 ipalib/plugins/internal.py |3 +++
 3 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/install/ui/details.js b/install/ui/details.js
index 50fccce4946b207999ef3f902b666a567b2b3e21..8e0edaab90d0e2ea1325761026a7d0403674e1b8 100644
--- a/install/ui/details.js
+++ b/install/ui/details.js
@@ -245,6 +245,7 @@ IPA.details_facet = function(spec) {
 var that = IPA.facet(spec);
 
 that.pre_execute_hook = spec.pre_execute_hook;
+that.post_update_hook = spec.post_update_hook;
 
 that.label = spec.label || IPA.messages  IPA.messages.facets  IPA.messages.facets.details;
 that.facet_group = spec.facet_group || 'settings';
@@ -526,7 +527,12 @@ IPA.details_facet = function(spec) {
 on_win(data, text_status, xhr);
 if (data.error)
 return;
-
+
+if (that.post_update_hook) {
+that.post_update_hook(data, text_status);
+return;
+}
+
 var result = data.result.result;
 that.load(result);
 }
diff --git a/install/ui/dns.js b/install/ui/dns.js
index bba1e5cbf06cc7f8f2694db475b20750ad5673e1..7cef5ae72eda85a41c143e517db4191fd26976d9 100644
--- a/install/ui/dns.js
+++ b/install/ui/dns.js
@@ -265,7 +265,17 @@ IPA.entity_factories.dnsrecord = function() {
 return IPA.entity_builder().
 entity('dnsrecord').
 containing_entity('dnszone').
-details_facet({
+details_facet({
+post_update_hook:function(data){
+var result = data.result.result;
+ if (result.idnsname) {
+this.load(result);
+} else {
+this.reset();
+var dialog = IPA.dnsrecord_redirection_dialog();
+dialog.open(this.container);
+}
+},
 disable_breadcrumb: false,
 sections:[
{
@@ -424,6 +434,28 @@ IPA.entity_factories.dnsrecord = function() {
 build();
 };
 
+IPA.dnsrecord_redirection_dialog = function(spec) {
+spec = spec || {};
+spec.title = spec.title || IPA.messages.dialogs.redirection;  
+
+var that = IPA.dialog(spec);
+
+that.create = function() {
+$('p/', {
+'text': IPA.messages.objects.dnsrecord.deleted_no_data
+}).appendTo(that.container);
+$('p/', {
+'text': IPA.messages.objects.dnsrecord.redirection_dnszone
+}).appendTo(that.container);
+};
+
+that.add_button(IPA.messages.buttons.ok, function() { 
+that.close();
+IPA.nav.show_page('dnszone','default');   
+});
+return that;
+};
+
 IPA.dnsrecord_host_link_widget = function(spec){
 var that = IPA.entity_link_widget(spec);
 that.other_pkeys = function(){
diff --git a/ipalib/plugins/internal.py b/ipalib/plugins/internal.py
index 6f279834a91092b67434bffaabb8d381db16319b..8556d941c35af658c5b7460d1edc7ebcd8d6975e 100644
--- a/ipalib/plugins/internal.py
+++ b/ipalib/plugins/internal.py
@@ -160,6 +160,8 @@ class i18n_messages(Command):
 dnsrecord: {
 type:_(Record Type),
 data:_(Data),
+deleted_no_data:_(DNS record was deleted because it contained no data.),
+redirection_dnszone:_(You will be redirected to DNS Zone.),
 title:_(Records for DNS Zone),
 },
 entitle: {
@@ -348,6 +350,7 @@ class i18n_messages(Command):
 dirty_message:_(This page has unsaved changes. Please save or revert.),
 dirty_title:_(Dirty),
 hide_already_enrolled:_(Hide already enrolled.),
+redirection:_(Redirection),
 remove_empty:_(Select entries to be removed.),
 remove_title:_(Remove ${entity}),
 prospective:_(Prospective),
-- 
1.7.6

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

[Freeipa-devel] [PATCH] 002 Fixed adding host without DNS reverse zone

2011-07-26 Thread Petr Vobornik
Fixed adding host without DNS reverse zone

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

Shows status dialog instead of error dialog (error 4304 is treated like
success).

This patch is fixing the problem, but maybe in a wrong way.

Main problem was that error has to be treated like success. This
decision is done in command.execute() method.

There are two ways to do it
1) Interrupt error handling - transform error to success
2) Interrupt success handling - don't let success to be transformed into
error.

Solution is using the second option. But I think first option is better.
But there are obstacles:
- handling is done in private function (for me ipa.js line ~ 290)
- there is an extend point - setting on_error method. Problem is that
this method is executed only if command.retry is false (default is
true). Setting it to false will disable usage of error dialog (which is
private function). So I would lose functionality for normal errors.
Reordering these lines isn't an option because it would affect a lot of
code.
- one way would be to extract code for error dialog and make it a
regular reusable dialog (with command as parameter). This way it can be
used in custom error handler.


Is it ACKable, or is it better to do it as described?

Petr



From 3601c857fd4425c3df998e66a39235b67c441813 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Tue, 26 Jul 2011 09:59:19 +0200
Subject: [PATCH] Fixed adding host without DNS reverse zone

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

Shows status dialog instead of error dialog (error 4304 is treated like success).
---
 install/ui/add.js|   10 +++---
 install/ui/dialog.js |   29 +
 install/ui/host.js   |   32 +++-
 install/ui/ipa.js|   13 +
 4 files changed, 80 insertions(+), 4 deletions(-)

diff --git a/install/ui/add.js b/install/ui/add.js
index 614a209056070cfa973c3cffa6cec935443ceb6e..9ebfb48bb65b0656bbfb1e3cd8f9c361b59c3d12 100644
--- a/install/ui/add.js
+++ b/install/ui/add.js
@@ -31,6 +31,8 @@ IPA.add_dialog = function (spec) {
 
 that.method = spec.method || 'add';
 that.pre_execute_hook = spec.pre_execute_hook;
+that.is_custom_success = spec.is_custom_success;
+that.on_custom_success = spec.on_custom_success;
 
 function show_edit_page(entity_name,result){
 var pkey_name = IPA.metadata.objects[entity_name].primary_key;
@@ -49,7 +51,7 @@ IPA.add_dialog = function (spec) {
 that.save(record);
 that.add(
 record,
-function(data, text_status, xhr) {
+function(data, text_status, xhr) {   
 var facet = IPA.current_entity.get_facet();
 var table = facet.table;
 table.refresh();
@@ -64,7 +66,7 @@ IPA.add_dialog = function (spec) {
 that.save(record);
 that.add(
 record,
-function(data, text_status, xhr) {
+function(data, text_status, xhr) {   
 var facet = IPA.current_entity.get_facet();
 var table = facet.table;
 table.refresh();
@@ -104,7 +106,9 @@ IPA.add_dialog = function (spec) {
 entity: that.entity_name,
 method: that.method,
 on_success: on_success,
-on_error: on_error
+on_error: on_error,
+is_custom_success: that.is_custom_success,
+on_custom_success: that.on_custom_success
 });
 
 pkey_prefix = IPA.get_entity(that.entity_name).get_primary_key_prefix();
diff --git a/install/ui/dialog.js b/install/ui/dialog.js
index ada30b0f481267899f2e1ee6fe4b784da9e1f4ba..f055e18b0b64fb39b15ded847213806227aaf0a8 100644
--- a/install/ui/dialog.js
+++ b/install/ui/dialog.js
@@ -660,3 +660,32 @@ IPA.deleter_dialog =  function (spec) {
 
 return that;
 };
+
+IPA.message_dialog = function(spec) {
+
+var that = IPA.dialog(spec);  
+
+var init = function(spec) {
+spec = spec || {};
+that.message = spec.message || '';
+that.on_ok = spec.on_ok;
+};
+that.message_dialog_init = init;
+
+that.create = function() {
+$('p/', {
+'text': that.message
+}).appendTo(that.container);   
+};
+  
+that.add_button(IPA.messages.buttons.ok, function() { 
+that.close();
+if(that.on_ok) {
+that.on_ok();
+}
+});  
+
+init(spec);
+
+return that;
+};
diff --git a/install/ui/host.js b/install/ui/host.js
index bbac4edd77c7528f4be97e5d50e24385e8bd5549..b7b5b6d910a54d5b9886707170ebb6db1da7109c 100644
--- a/install/ui/host.js
+++ b/install/ui/host.js
@@ -102,6 +102,7 @@ IPA.entity_factories.host = function () {
 }).
 standard_association_facets

Re: [Freeipa-devel] [PATCH] 002 Fixed adding host without DNS reverse zone

2011-07-27 Thread Petr Vobornik
On Tue, 2011-07-26 at 21:32 -0400, Adam Young wrote: 
 On 07/26/2011 07:09 PM, Endi Sukma Dewata wrote:
  On 7/26/2011 6:27 AM, Petr Vobornik wrote:
  Fixed adding host without DNS reverse zone
 
  https://fedorahosted.org/freeipa/ticket/1481
 
  Shows status dialog instead of error dialog (error 4304 is treated like
  success).
 
  This patch is fixing the problem, but maybe in a wrong way.
 
  Main problem was that error has to be treated like success. This
  decision is done in command.execute() method.
 
  There are two ways to do it
  1) Interrupt error handling - transform error to success
  2) Interrupt success handling - don't let success to be transformed into
  error.
 
  Solution is using the second option. But I think first option is better.
  But there are obstacles:
  - handling is done in private function (for me ipa.js line ~ 290)
  - there is an extend point - setting on_error method. Problem is that
  this method is executed only if command.retry is false (default is
  true). Setting it to false will disable usage of error dialog (which is
  private function). So I would lose functionality for normal errors.
  Reordering these lines isn't an option because it would affect a lot of
  code.
  - one way would be to extract code for error dialog and make it a
  regular reusable dialog (with command as parameter). This way it can be
  used in custom error handler.
 
 
  Is it ACKable, or is it better to do it as described?
 
  Petr
 
  Hi Petr,
 
  The new is_custom_success and on_custom_success attributes in 
  IPA.command somehow competes with the original on_success because they 
  serve a similar purpose. I think it's better to make the default error 
  dialog in IPA.command public so it can be used by other code as well.
 
  We have a global variable IPA.error_dialog which stores the DOM 
  element for the error dialog. I think we can convert it into a global 
  object which you can open/close to show the default error dialog. The 
  original DOM element can be stored in a 'container' attribute in that 
  object.
 
  In other words, convert dialog_open() into IPA.error_dialog.open(), 
  move the original IPA.error_dialog into IPA.error_dialog.container. 
  Set retry to false when invoking IPA.command, then specify an error 
  handler which will catch error 4304. For other errors you'll display 
  the default error dialog.
 
  There are also some warnings about trailing whitespaces when applying 
  the patch. You can remove them by adding the --whitespace=fix option 
  when applying the patch with git am.
 
 On the whitespace issue, if you are an emacs person, there is a 
 command:  alt-x whitespace-cleanup that you should run on a file after 
 you make changes.
 
 
 I have
   '(show-trailing-whitespace t))
 in my .emacs file, which shows all whitespace as red...which properly 
 motivates you to clean it up as soon as possible.  I'm not sure the 
 comparable vi settings, but I know they exist.
 
 ___
 Freeipa-devel mailing list
 Freeipa-devel@redhat.com
 https://www.redhat.com/mailman/listinfo/freeipa-devel

Reworked.

-Refactored error dialog.
-Changed context of calling command.on_success and command.on_error
methods from $.ajax's object to command.
-Added generic message dialog (IPA.message_dialog) (not changed form
previous)

Should be without trailing whitespaces. :)

Petr


From bbd92b1febed95a0b3782c0565d2e247ec31ece9 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Tue, 26 Jul 2011 09:59:19 +0200
Subject: [PATCH] Fixed adding host without DNS reverse zone

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

Shows status dialog instead of error dialog (error 4304 is treated like success).

Refactored error dialog.
Changed context of calling command.on_success and command.on_error methods from $.ajax's object to command.
Added generic message dialog (IPA.message_dialog)
---
 install/ui/add.js|   16 ---
 install/ui/dialog.js |   29 
 install/ui/host.js   |   42 +
 install/ui/ipa.js|  121 --
 4 files changed, 149 insertions(+), 59 deletions(-)

diff --git a/install/ui/add.js b/install/ui/add.js
index 614a209056070cfa973c3cffa6cec935443ceb6e..9ca052d6b0b05837b560189de464651da720d8b6 100644
--- a/install/ui/add.js
+++ b/install/ui/add.js
@@ -31,6 +31,9 @@ IPA.add_dialog = function (spec) {
 
 that.method = spec.method || 'add';
 that.pre_execute_hook = spec.pre_execute_hook;
+that.on_error = spec.on_error;
+
+that.retry = typeof spec.retry !== 'undefined' ? spec.retry : true;
 
 function show_edit_page(entity_name,result){
 var pkey_name = IPA.metadata.objects[entity_name].primary_key;
@@ -54,8 +57,8 @@ IPA.add_dialog = function (spec) {
 var table = facet.table;
 table.refresh();
 that.close

Re: [Freeipa-devel] [PATCH] 002 Fixed adding host without DNS reverse zone

2011-07-29 Thread Petr Vobornik
There was a small error in add.js:162. Fixed!

On Fri, 2011-07-29 at 11:00 -0400, Adam Young wrote:
 On 07/29/2011 10:58 AM, Adam Young wrote: 
  Due to my recent huge patch, version -1  patch will not apply.  I
  had to rebase by hand.
  
  Please confirm that it still works as intended.
 
 
 Missed a few files in my commit. 
 ___
 Freeipa-devel mailing list
 Freeipa-devel@redhat.com
 https://www.redhat.com/mailman/listinfo/freeipa-devel

From 75cc2819fafefc19d3feec7daf63b5bbe0aad4ca Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Fri, 29 Jul 2011 10:53:01 -0400
Subject: [PATCH] Fixed adding host without DNS reverse zone

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

Shows status dialog instead of error dialog (error 4304 is treated like success).

Refactored error dialog.
Changed context of calling command.on_success and command.on_error methods from $.ajax's object to command.
Added generic message dialog (IPA.message_dialog)
---
 install/ui/add.js|   15 ---
 install/ui/dialog.js |   29 
 install/ui/host.js   |   42 +
 install/ui/ipa.js|  121 --
 4 files changed, 148 insertions(+), 59 deletions(-)

diff --git a/install/ui/add.js b/install/ui/add.js
index 988ea8ff13819ccdd61a2033344e146dbaf09255..b9a22468639fa86a27cb1cd522ad96a785f2eea1 100644
--- a/install/ui/add.js
+++ b/install/ui/add.js
@@ -31,6 +31,8 @@ IPA.add_dialog = function (spec) {
 
 that.method = spec.method || 'add';
 that.pre_execute_hook = spec.pre_execute_hook;
+that.on_error = spec.on_error ;
+that.retry = typeof spec.retry !== 'undefined' ? spec.retry : true;
 
 function show_edit_page(entity,result){
 var pkey_name = entity.metadata.primary_key;
@@ -51,6 +53,7 @@ IPA.add_dialog = function (spec) {
 var command = IPA.command({
 entity: that.entity.name,
 method: that.method,
+retry: that.retry,
 on_success: on_success,
 on_error: on_error
 });
@@ -127,8 +130,8 @@ IPA.add_dialog = function (spec) {
 var table = facet.table;
 table.refresh();
 that.close();
-}
-);
+},
+that.on_error);
 });
 
 that.add_button(IPA.messages.buttons.add_and_add_another, function() {
@@ -141,8 +144,8 @@ IPA.add_dialog = function (spec) {
 var table = facet.table;
 table.refresh();
 that.reset();
-}
-);
+},
+that.on_error);
 });
 
 that.add_button(IPA.messages.buttons.add_and_edit, function() {
@@ -154,8 +157,8 @@ IPA.add_dialog = function (spec) {
 that.close();
 var result = data.result.result;
 that.show_edit_page(that.entity,result);
-}
-);
+},
+that.on_error);
 });
 
 that.add_button(IPA.messages.buttons.cancel, function() {
diff --git a/install/ui/dialog.js b/install/ui/dialog.js
index 848252d87f4db8418f26ec5c7dfebbfaca5f0275..ad95eceda97fdbf5e93af2dd77de0ab12963f2f3 100644
--- a/install/ui/dialog.js
+++ b/install/ui/dialog.js
@@ -644,3 +644,32 @@ IPA.deleter_dialog =  function (spec) {
 
 return that;
 };
+
+IPA.message_dialog = function(spec) {
+
+var that = IPA.dialog(spec);
+
+var init = function(spec) {
+spec = spec || {};
+that.message = spec.message || '';
+that.on_ok = spec.on_ok;
+};
+that.message_dialog_init = init;
+
+that.create = function() {
+$('p/', {
+'text': that.message
+}).appendTo(that.container);
+};
+
+that.add_button(IPA.messages.buttons.ok, function() {
+that.close();
+if(that.on_ok) {
+that.on_ok();
+}
+});
+
+init(spec);
+
+return that;
+};
diff --git a/install/ui/host.js b/install/ui/host.js
index a84f54c190257e19efadcbdf0754b431eb4bd6de..8a40f07b18b20396b537f6d8fac6fe7f3d541e0c 100644
--- a/install/ui/host.js
+++ b/install/ui/host.js
@@ -102,6 +102,7 @@ IPA.entity_factories.host = function () {
 }).
 standard_association_facets().
 adder_dialog({
+factory: IPA.host_adder_dialog,
 width: 400,
 height: 250,
 fields:[
@@ -128,6 +129,47 @@ IPA.entity_factories.host = function () {
 build();
 };
 
+IPA.host_adder_dialog = function(spec)
+{
+spec = spec || {};
+spec.retry = typeof spec.retry !== 'undefined' ? spec.retry : false;
+
+var that = IPA.add_dialog(spec);
+
+that.on_error = function(xhr, text_status, error_thrown)
+{
+var command = this;
+var data = error_thrown.data;
+var dialog = null;
+
+if(data  data.error  data.error.code === 4304) {
+dialog = IPA.message_dialog({
+message

Re: [Freeipa-devel] [PATCH] 843 reduce dogtag install time

2011-08-03 Thread Petr Vobornik
On Mon, 2011-08-01 at 23:03 -0400, Adam Young wrote:
 On 08/01/2011 10:26 PM, Adam Young wrote: 
  On 08/01/2011 03:19 PM, Rob Crittenden wrote: 
   Ade Lee from the dogtag team looked at our installer and found
   that we restarted the pki-cad process too many times. Re-arranging
   some code allows us to restart it just once. The new config time
   for dogtag is 3 1/2 minutes, down from about 5 1/2. 
   
   Ade is working on improvements in pki-silent as well which can
   bring the overall install time to 90 seconds. If we can get a
   change in SELinux policy we're looking at 60 seconds. 
   
   This patch just contains the reworked installer part. Once an
   updated dogtag is released we can update the spec file to pull it
   in. 
   
   rob 
   
   ___
   Freeipa-devel mailing list
   Freeipa-devel@redhat.com
   https://www.redhat.com/mailman/listinfo/freeipa-devel
  
 
 Disregard:  same thing seems to be happening without this patch.
 
  
  Something is wrong.  When I installed this patch, the browser works
  fine in a clean mode (never before initiailzied).  Howevr, if the
  browser already has a certificate from the server, in the past I was
  able to go into  Edit-preferences-advanced-Certificates, and
  remove both the server and the CA certificate, and then restart the
  browser.  That does not work now.  I just get the message
  
  Secure Connection Failed
  An error occurred during a connection to
  server15.ayoung.boston.devel.redhat.com.
  
  You have received an invalid certificate.  Please contact the server
  administrator or email correspondent and give them the following
  information:
  
  Your certificate contains the same serial number as another
  certificate issued by the certificate authority.  Please get a new
  certificate containing a unique serial number.
  
  (Error code: sec_error_reused_issuer_and_serial)  
  
The page you are trying to view can not be shown because the
  authenticity of the received data could not be verified.
Please contact the web site owners to inform them of this problem.
  Alternatively, use the command found in the help menu to report this
  broken site.
  
  
  Restarting IPA made no difference.  The browser does not provide a
  lot of info in which to debug this.
  
  
  I'll try again with out the patch and see if there is a difference.
  

In Firefox 5 I also have to clear browser cache along with removing
certificates to get rid of 'sec_error_reused_issuer_and_serial'.

Petr


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


Re: [Freeipa-devel] [PATCH] 002 Fixed adding host without DNS reverse zone

2011-08-04 Thread Petr Vobornik
new version attached

On Fri, 2011-07-29 at 12:11 -0500, Endi Sukma Dewata wrote: 
 On 7/29/2011 11:12 AM, Petr Vobornik wrote:
  There was a small error in add.js:162. Fixed!
 
 Nice job on the dialog boxes.
 
 There's a problem though, the Retry doesn't quite work. This is because 
 'this' object passed to IPA.error_dialog actually points to Ajax context 
 instead of the IPA.command, so calling execute() on it will fail.
Fixed 
 
 When Ajax call returns, it passes a context via 'this' object to the 
 callback function. The object might contain some useful information 
 which we would not be able to get any other way. The original code tries 
 to maintain the context by passing 'this' object along the chain using 
 call(). Feel free to add comments in the code to clarify this.
 
 So in dialog_open() you should pass 'that' into the 'command' parameter. 
 You also need pass 'this' using another parameter so you can use it to 
 call the error handler if you click Cancel.
 
 Also these changes should be reverted back to maintain the Ajax context:
 
 - that.on_error.call(this, xhr, text_status, error_thrown);
 + that.on_error(xhr, text_status, error_thrown);
 
 - that.on_success.call(this, data, text_status, xhr);
 + that.on_success(data, text_status, xhr);

Reverted back. Just for my information: ajax context is preserved for
some future use, or it is already used somewhere?

 The IPA.add_dialog can store the command object as an instance variable 
 so the IPA.host_adder_dialog can refer to it from the error handler.
 
 Another thing, in the init() you can access the spec object directly, so 
 don't really have to pass it as a parameter.
Yeah, I know. The purpose for this was to be able to call init method
again later (which was made public as xxx_init(spec)). But probably it
isn't in compliance with removes of public init methods. 

petr

From e142dc9764c8370957888081ebc6bafc611917e7 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Fri, 29 Jul 2011 10:53:01 -0400
Subject: [PATCH] Fixed adding host without DNS reverse zone

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

Shows status dialog instead of error dialog (error 4304 is treated like success).

Refactored error dialog.
Added generic message dialog (IPA.message_dialog)
---
 install/ui/add.js|   17 +---
 install/ui/dialog.js |   28 
 install/ui/host.js   |   42 ++
 install/ui/ipa.js|  119 -
 4 files changed, 149 insertions(+), 57 deletions(-)

diff --git a/install/ui/add.js b/install/ui/add.js
index 988ea8ff13819ccdd61a2033344e146dbaf09255..b4f1228f04d51893598860261b3bda09d07f8fab 100644
--- a/install/ui/add.js
+++ b/install/ui/add.js
@@ -31,6 +31,9 @@ IPA.add_dialog = function (spec) {
 
 that.method = spec.method || 'add';
 that.pre_execute_hook = spec.pre_execute_hook;
+that.on_error = spec.on_error ;
+that.retry = typeof spec.retry !== 'undefined' ? spec.retry : true;
+that.command = null;
 
 function show_edit_page(entity,result){
 var pkey_name = entity.metadata.primary_key;
@@ -51,9 +54,11 @@ IPA.add_dialog = function (spec) {
 var command = IPA.command({
 entity: that.entity.name,
 method: that.method,
+retry: that.retry,
 on_success: on_success,
 on_error: on_error
 });
+that.command = command;
 
 pkey_prefix = that.entity.get_primary_key_prefix();
 
@@ -127,8 +132,8 @@ IPA.add_dialog = function (spec) {
 var table = facet.table;
 table.refresh();
 that.close();
-}
-);
+},
+that.on_error);
 });
 
 that.add_button(IPA.messages.buttons.add_and_add_another, function() {
@@ -141,8 +146,8 @@ IPA.add_dialog = function (spec) {
 var table = facet.table;
 table.refresh();
 that.reset();
-}
-);
+},
+that.on_error);
 });
 
 that.add_button(IPA.messages.buttons.add_and_edit, function() {
@@ -154,8 +159,8 @@ IPA.add_dialog = function (spec) {
 that.close();
 var result = data.result.result;
 that.show_edit_page(that.entity,result);
-}
-);
+},
+that.on_error);
 });
 
 that.add_button(IPA.messages.buttons.cancel, function() {
diff --git a/install/ui/dialog.js b/install/ui/dialog.js
index 0a1d5428a5b1763cbf4871a9cd59e7415f309c79..dc23db8d04a311d386f44c65c58aacee70c9a13c 100644
--- a/install/ui/dialog.js
+++ b/install/ui/dialog.js
@@ -656,3 +656,31 @@ IPA.deleter_dialog =  function (spec) {
 
 return that;
 };
+
+IPA.message_dialog = function(spec) {
+
+var that = IPA.dialog(spec);
+
+var init = function() {
+spec = spec || {};
+that.message = spec.message || '';
+that.on_ok = spec.on_ok

Re: [Freeipa-devel] [PATCH] 002 Fixed adding host without DNS reverse zone

2011-08-05 Thread Petr Vobornik
On Thu, 2011-08-04 at 13:24 -0500, Endi Sukma Dewata wrote: 
 On 8/4/2011 4:22 AM, Petr Vobornik wrote:
  new version attached
 
 Almost there, just a few more minor issues.
 
  Also these changes should be reverted back to maintain the Ajax context:
 
  - that.on_error.call(this, xhr, text_status, error_thrown);
  + that.on_error(xhr, text_status, error_thrown);
 
  - that.on_success.call(this, data, text_status, xhr);
  + that.on_success(data, text_status, xhr);
 
  Reverted back. Just for my information: ajax context is preserved for
  some future use, or it is already used somewhere?
 
 The Ajax context right now is only used to get the URL causing HTTP 
 error (ipa.js:301). Things might have changed, I'm not sure how to 
 generate HTTP error anymore. The URL can actually be obtained from the 
 url variable in the execute() method, but there are other things that 
 you can get from Ajax context that might be useful in the future. Try 
 setting a breakpoint inside the success_handler() or error_handler() and 
 inspect the 'this' variable. I think we should make sure the callback 
 functions behave like real Ajax callback function to avoid future 
 problems, so 'this' should always point to Ajax context.
 
 There are actually a few places where the Ajax context doesn't get 
 passed to the callback function:
   - ipa.js:409,418,428,431,436,620
   - host.js:155
 A bunch of these are existing issues. We can fix them separately.
 
  Another thing, in the init() you can access the spec object directly, so
  don't really have to pass it as a parameter.
 
  Yeah, I know. The purpose for this was to be able to call init method
  again later (which was made public as xxx_init(spec)). But probably it
  isn't in compliance with removes of public init methods.
 
 The init() method that we removed recently was a method that was called 
 to initialize the object after the metadata becomes available. In the 
 past some objects were created before the metadata was available, but 
 now it's no longer the case so the object can be created and initialized 
 at the same time. There's nothing wrong with creating an init() method 
 to encapsulate the initialization sequence, but it doesn't need to be 
 made public like before because the subclasses no longer need to call it 
 explicitly. No need to change anything here.
 
 The default values in ipa.js:576-579 are redundant because they will be 
 overridden by the spec in init().
Removed. 
 I think the assignments in init() can 
 be replaced by something like this:
  that.xhr = spec.xhr || {};
 Note that the default value for xhr and error_thrown should be an empty 
 object.
Reworked, probably we should add some generic error title to internal.py
as default value for error dialog title. 
 
 There are some unit test failures in ipa_tests.js because 
 IPA.error_dialog used to point to the dialog instance. You might want to 
 change it to get the instance using something else, e.g. element ID.

- Added property 'id' to dialog (which is added to its div)
- Added reference to ../dialog.js in ipa.tests.html
- Reworked ipa.test.js to work with error_dialog id.

 
 There are some other other unit test failures, but they seem to be 
 caused by the earlier failure. They actually pass if run separately.
 

-- All test should pass.

-- 
Petr Voborník 
From d765145a21e9cbf1b1b6afc2340470f87c179374 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Fri, 5 Aug 2011 17:12:21 +0200
Subject: [PATCH] Fixed adding host without DNS reverse zone

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

Shows status dialog instead of error dialog (error 4304 is treated like success).

Refactored error dialog.
Added generic message dialog (IPA.message_dialog)
Modified core tests to work with dialog.
---
 install/ui/add.js  |   17 --
 install/ui/dialog.js   |   32 +++-
 install/ui/host.js |   42 +++
 install/ui/ipa.js  |  115 ++--
 install/ui/test/ipa_tests.html |1 +
 install/ui/test/ipa_tests.js   |   23 +---
 6 files changed, 163 insertions(+), 67 deletions(-)

diff --git a/install/ui/add.js b/install/ui/add.js
index 988ea8ff13819ccdd61a2033344e146dbaf09255..b4f1228f04d51893598860261b3bda09d07f8fab 100644
--- a/install/ui/add.js
+++ b/install/ui/add.js
@@ -31,6 +31,9 @@ IPA.add_dialog = function (spec) {
 
 that.method = spec.method || 'add';
 that.pre_execute_hook = spec.pre_execute_hook;
+that.on_error = spec.on_error ;
+that.retry = typeof spec.retry !== 'undefined' ? spec.retry : true;
+that.command = null;
 
 function show_edit_page(entity,result){
 var pkey_name = entity.metadata.primary_key;
@@ -51,9 +54,11 @@ IPA.add_dialog = function (spec) {
 var command = IPA.command({
 entity: that.entity.name,
 method: that.method,
+retry: that.retry,
 on_success: on_success

Re: [Freeipa-devel] [PATCH] 238 Fixed error after login on IE

2011-08-09 Thread Petr Vobornik

On 08/08/2011 08:34 PM, Endi Sukma Dewata wrote:

The IE does not resend the request body during negotiation, so after
after a successful authentication the server could not find the JSON
request to parse.

The Web UI has been modified to detect this error and resend the
initialization request.

Ticket #1540



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

Code looks good.

But, I didn't test it in IE9, because I don't have one nor Windows vm 
(will try to get one).


Modifications in error handling are OK.

If it really works in IE I would consider it ACKed.

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 239 Fixed host adder dialog.

2011-08-09 Thread Petr Vobornik

On 08/08/2011 11:13 PM, Endi Sukma Dewata wrote:

The host adder dialog has been modified to use separate fields for
hostname and DNS zone. The hostname is a text field and the DNS zone
is an editable drop-down list. The fields will have the following
behavior:

- If the user types a dot into the hostname field, the cursor will
automatically move into the DNS zone field.
- If the user pastes an FQDN into the hostname field, the value will
automatically be split into hostname and DNS zone.
- If the user selects a value from the drop-down list, it will only
change the DNS zone, not the hostname.

Ticket #1457



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

NACK

Add and edit without existing DNS reverse zone don't redirect to edit 
page.


Field 'fqdn' doesn't exist any more so getting fqdn from field in 
host.js:280 fqdn: that.get_field('fqdn').save() isn't functional.


--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 239 Fixed host adder dialog.

2011-08-10 Thread Petr Vobornik

On 08/09/2011 08:04 PM, Endi Sukma Dewata wrote:

On 8/9/2011 7:12 AM, Petr Vobornik wrote:

NACK

Add and edit without existing DNS reverse zone don't redirect to
edit page.

Field 'fqdn' doesn't exist any more so getting fqdn from field in
host.js:280 fqdn: that.get_field('fqdn').save() isn't functional.


Fixed. Now the fqdn is obtained from the command object.

On 8/8/2011 9:17 PM, Adam Young wrote:

The JQuery code drawing the table in the create method is
cut-and-paste.. It should be possible to make it look right without
redrawing the whole table.

These two fields should be put into their own section, which can then
be responsible for drawing just the rows responsible for these
fields, leaving the default behavior for the other rows.


As discussed, this will be done separately in this ticket:
https://fedorahosted.org/freeipa/ticket/1394



ACK

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 240 Fixed DNS zone adder dialog.

2011-08-10 Thread Petr Vobornik

On 08/09/2011 10:09 PM, Endi Sukma Dewata wrote:

The DNS zone adder dialog has been modified to use radio buttons to
select whether to enter a zone name or a reverse zone IP network.

Ticket #1575



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


ACK

Same repetitive code as in host adder dialog (1457). But it should be 
covered by https://fedorahosted.org/freeipa/ticket/1394.


--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 241 Fixed broken links in ipa_error.css and ipa_migration.css.

2011-08-11 Thread Petr Vobornik

On 08/10/2011 04:21 AM, Endi Sukma Dewata wrote:

Some of the images that were previously deleted are actually needed
by ipa_error.css and ipa_migration.css, so they have been restored.

Ticket #1564



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

ACK

I don't like that there is no pattern in image file naming. Sometimes it 
uses dashes, sometimes underscores. Some file names are capitalized. But 
this isn't the subject of this patch (ticket).


--
Petr Vobornik

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


[Freeipa-devel] [PATCH] 004 error dialog for batch command

2011-08-11 Thread Petr Vobornik

[PATCH] error dialog for batch command

https://fedorahosted.org/freeipa/ticket/1597
https://fedorahosted.org/freeipa/ticket/1592

Added option to show multiple errors in error dialog.

Notes:
- also covering '[ipa webui] Does not return appropriate error when 
deleting an external host but checking update dns' (1592)

- added support(element's classes) for css styling of aggregated errors
- except search dialog delete (1592) - no other batch command uses this 
feature (has to be explicitly turned on).


--
Petr Vobornik
From 52254b91e3c2546fea01c18f8bc879316ae7bb3d Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Thu, 11 Aug 2011 10:28:50 +0200
Subject: [PATCH] error dialog for batch command

https://fedorahosted.org/freeipa/ticket/1597
https://fedorahosted.org/freeipa/ticket/1592

Added option to show multiple errors in error dialog.
---
 install/ui/ipa.js  |  125 ---
 install/ui/search.js   |5 +-
 install/ui/test/data/ipa_init.json |1 +
 ipalib/plugins/internal.py |1 +
 4 files changed, 120 insertions(+), 12 deletions(-)

diff --git a/install/ui/ipa.js b/install/ui/ipa.js
index 8a3dd4e7d596914687e412aefdda27d7d699261d..e9de88b9ca1235d4bc6c6115dc74acdb0d882348 100644
--- a/install/ui/ipa.js
+++ b/install/ui/ipa.js
@@ -417,6 +417,10 @@ IPA.batch_command = function (spec) {
 var that = IPA.command(spec);
 
 that.commands = [];
+that.errors = [];
+that.partial_success_message = spec.partial_success_message || '';
+that.partial_success_notify = typeof spec.partial_success_notify == 'undefined' ?
+false : spec.partial_success_notify;
 
 that.add_command = function(command) {
 that.commands.push(command);
@@ -429,7 +433,21 @@ IPA.batch_command = function (spec) {
 }
 };
 
+var clear_errors = function() {
+that.errors = [];
+};
+
+var add_error = function(command, name, message, status) {
+that.errors.push({
+command: command,
+name: name,
+message: message,
+status: status
+});
+};
+
 that.execute = function() {
+clear_errors();
 
 IPA.command({
 name: that.name,
@@ -444,25 +462,38 @@ IPA.batch_command = function (spec) {
 var command = that.commands[i];
 var result = data.result.results[i];
 
+var name = '';
+var message = '';
+
 if (!result) {
+name = 'Internal Error '+xhr.status;
+message = result ? xhr.statusText : Internal error;
+
+add_error(command, name, message, text_status);
+
 if (command.on_error) command.on_error.call(
 this,
 xhr,
 text_status,
 {
-name: 'Internal Error '+xhr.status,
-message: result ? xhr.statusText : Internal error
+name: name,
+message: message
 }
 );
 
 } else if (result.error) {
+name = 'IPA Error ' + (result.error.code || '');
+message = result.error.message || result.error;
+
+add_error(command, name, message, text_status);
+
 if (command.on_error) command.on_error.call(
 this,
 xhr,
 text_status,
 {
-name: 'IPA Error '+result.error.code,
-message: result.error.message
+name: name,
+message: message
 }
 );
 
@@ -470,6 +501,21 @@ IPA.batch_command = function (spec) {
 if (command.on_success) command.on_success.call(this, result, text_status, xhr);
 }
 }
+//check for partial errors and show error dialog
+if(that.partial_success_notify  that.errors.length  0) {
+var dialog = IPA.error_dialog({
+xhr: xhr,
+text_status: text_status,
+error_thrown: {
+name: that.partial_success_message,
+message: that.partial_success_message
+},
+command: that,
+errors: that.errors,
+visible_buttons: ['ok']
+});
+dialog.open

Re: [Freeipa-devel] [PATCH] 245 Fixed problem with buttons in enrollment dialog.

2011-08-15 Thread Petr Vobornik

On 08/15/2011 04:29 PM, Endi Sukma Dewata wrote:

The panel for selection buttons (i.e.  and ) has been re-
positioned to avoid being covered by the adder-dialog-right panel.

Ticket #1626



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

ACK

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 004 error dialog for batch command

2011-08-15 Thread Petr Vobornik

On 08/11/2011 06:16 PM, Endi Sukma Dewata wrote:

On 8/11/2011 11:03 AM, Endi Sukma Dewata wrote:

Some issues:

1. I think by default all batch commands should use this feature. The
batch command is used for various purposes, not just for deletion.
Consider this scenario:

First, find a way to log in simultaneously using different accounts. You
can use either multiple machines, accounts, or browsers, whichever is
the easiest.

In the first session, log in as admin, create a test user, add it into
the admins group.

Then in the second session, login as the test user, then edit a sudo
rule. Modify the description and the enabled flag (this will be executed
as separate operations in a single batch). Don't click Update yet.

Back to the first session, remove the test user from the admins group.
Then go back to the second session, click Update.

Since the test user doesn't have admin rights anymore the operations
will fail. However, currently these failures are not reported and the
values simply revert back to the original. The error dialog should show
these errors.

So in this case we don't really need the 'partial_success_notify' flag,
or it can be renamed into 'show_error' which should be true by default.

done

The 'retry' flag in IPA.command can be renamed to 'show_error' too.
sending without this (not essential part of this patch - not 
batch_command). But I agree it should be probably changed - to be 
consistent.


2. The 'partial_success_message' probably can be renamed intofile:
'error_message' which will say something like 'Some operations failed.'

done


3. Instead of a checkbox for show_errors_checkbox, it might be better to
use 'Show details' and 'Hide details' links.

done


4. In ipa.js:510 instead of repeating the error message the
error_thrown.name could say something like 'Batch Error' or 'Operations
Error'.
used 'Operations Error' - batch is internal naming and probably 
confusing to user


5. The add_error() could be moved into IPA.error_dialog so the
IPA.batch_command doesn't need to hold the 'errors' list.

left in batch_command for possible future use in custom handlers


6. The list of errors should be displayed as a list (with bullets) like
in the deleter dialog.

done

7. Just for consistency, the Retry label declaration in ipa.js:737 could
be moved into the if-statement like the other labels.


done


--
Petr Vobornik
From 383cc50aa31b24921b426eb662563be0f6943894 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Thu, 11 Aug 2011 10:28:50 +0200
Subject: [PATCH] error dialog for batch command

https://fedorahosted.org/freeipa/ticket/1597
https://fedorahosted.org/freeipa/ticket/1592

Added option to show multiple errors in error dialog.
---
 install/ui/ipa.js  |  138 ---
 install/ui/search.js   |4 +-
 install/ui/test/data/ipa_init.json |7 ++-
 ipalib/plugins/internal.py |5 ++
 4 files changed, 140 insertions(+), 14 deletions(-)

diff --git a/install/ui/ipa.js b/install/ui/ipa.js
index 8a3dd4e7d596914687e412aefdda27d7d699261d..9a252f1e50fdaf544cb872fe0dbed9377e791559 100644
--- a/install/ui/ipa.js
+++ b/install/ui/ipa.js
@@ -417,6 +417,11 @@ IPA.batch_command = function (spec) {
 var that = IPA.command(spec);
 
 that.commands = [];
+that.errors = [];
+that.error_message = spec.error_message || (IPA.messages.dialogs ?
+IPA.messages.dialogs.batch_error_message : 'Some operations failed.');
+that.show_error = typeof spec.show_error == 'undefined' ?
+true : spec.show_error;
 
 that.add_command = function(command) {
 that.commands.push(command);
@@ -429,7 +434,21 @@ IPA.batch_command = function (spec) {
 }
 };
 
+var clear_errors = function() {
+that.errors = [];
+};
+
+var add_error = function(command, name, message, status) {
+that.errors.push({
+command: command,
+name: name,
+message: message,
+status: status
+});
+};
+
 that.execute = function() {
+clear_errors();
 
 IPA.command({
 name: that.name,
@@ -444,25 +463,38 @@ IPA.batch_command = function (spec) {
 var command = that.commands[i];
 var result = data.result.results[i];
 
+var name = '';
+var message = '';
+
 if (!result) {
+name = 'Internal Error '+xhr.status;
+message = result ? xhr.statusText : Internal error;
+
+add_error(command, name, message, text_status);
+
 if (command.on_error) command.on_error.call(
 this,
 xhr,
 text_status,
 {
-name: 'Internal Error '+xhr.status

Re: [Freeipa-devel] [PATCH] 242 Removed custom layout for password reset.

2011-08-15 Thread Petr Vobornik

On 08/11/2011 07:44 PM, Endi Sukma Dewata wrote:

The dialog box for resetting user password has been modified to use
the standard layout.



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

patch from code and working perspective is OK.

Has this patch assigned some ticket? Don't know exactly what is IPA's 
policy for submitting patches without tickets in Trac (for future tracking).


--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 247 Hide activation/deactivation link from regular users.

2011-08-17 Thread Petr Vobornik

On 08/16/2011 11:49 PM, Endi Sukma Dewata wrote:

The IPA.user_status_widget has been modified to show/hide the link for
activating/deactivating users according to the attributelevelrights.

Ticket #1625



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

ACK

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 246 Fixed browser configuration pages

2011-08-17 Thread Petr Vobornik

On 08/16/2011 11:20 PM, Endi Sukma Dewata wrote:

The browser configuration pages have been modified to improve the
content and appearance.

Ticket #1624



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

ACK

--
Petr Vobornik

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


[Freeipa-devel] [PATCH] 005 Show error in serial association

2011-08-17 Thread Petr Vobornik

Ticket #1628 - https://fedorahosted.org/freeipa/ticket/1628
Unreported insufficient access error

This patch is dependant on 
freeipa-pvoborni-0004-1-error-dialog-for-batch-command.patch.


This may be only a checking if approach of this patch is good.

I was not sure if this type of error message (result.failed property) is 
more general or it only appears in adding members. So I put error 
handling in serial_associator instead of command. If it would be put in 
command and success will be transformed to error, it will change the 
behaviour of executing commands - other commands after error won't be 
executed. If the approach is good, it could be probably better to change 
it a little and offer same logic for batch_associator.


It should be working for adding users to groups, netgroups, roles and 
assigning hbac rules (tested as non admin user).



Modified association test - data in success handler should not be undefined.

--
Petr Vobornik
From 7670faa42492e8850cffd7c54d60f6f6eaa7c51b Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Wed, 17 Aug 2011 15:06:41 +0200
Subject: [PATCH] Show error in serial association

https://fedorahosted.org/freeipa/ticket/1628
---
 install/ui/association.js|   43 ++--
 install/ui/ipa.js|   45 +++--
 install/ui/test/association_tests.js |2 +-
 3 files changed, 67 insertions(+), 23 deletions(-)

diff --git a/install/ui/association.js b/install/ui/association.js
index 2c6a1d2003be0668b61b230b8317263b06a822a5..ebc8ab3924633d038a6d21af50c9e8865968aeec 100644
--- a/install/ui/association.js
+++ b/install/ui/association.js
@@ -57,16 +57,18 @@ IPA.serial_associator = function(spec) {
 
 var that = IPA.associator(spec);
 
+that.errors = IPA.errors();
+
 that.execute = function() {
 
 if (!that.values || !that.values.length) {
-that.on_success();
+that.post_execute();
 return;
 }
 
 var value = that.values.shift();
 if (!value) {
-that.on_success();
+that.post_execute();
 return;
 }
 
@@ -79,7 +81,9 @@ IPA.serial_associator = function(spec) {
 method: that.method,
 args: args,
 options: options,
-on_success: that.execute,
+on_success: function(data, text_status, xhr) {
+that.command_success.call(this, command, data, text_status, xhr);
+},
 on_error: that.on_error
 });
 
@@ -88,6 +92,39 @@ IPA.serial_associator = function(spec) {
 command.execute();
 };
 
+that.command_success = function(command, data, text_status, xhr) {
+var result = data.result;
+if(result  result.failed) {
+for(var association in result.failed) {
+for(var member_name in result.failed[association]) {
+var member = result.failed[association][member_name];
+if(member.length  0  member[0].length  1) {
+var name = 'Association Error';
+var message = member[0][1];
+that.errors.add_error(command, name, message, text_status);
+}
+}
+}
+}
+that.execute();
+};
+
+that.post_execute = function () {
+if(that.errors.errors.length  0) {
+var dialog = IPA.error_dialog({
+error_thrown: {
+name: IPA.messages.dialogs.batch_error_title,
+message: IPA.messages.dialogs.batch_error_message
+},
+errors: that.errors.errors,
+visible_buttons: ['ok']
+});
+dialog.open();
+}
+
+that.on_success();
+};
+
 return that;
 };
 
diff --git a/install/ui/ipa.js b/install/ui/ipa.js
index 9a252f1e50fdaf544cb872fe0dbed9377e791559..142a3210a3af16df415c693277135fed1625eeeb 100644
--- a/install/ui/ipa.js
+++ b/install/ui/ipa.js
@@ -417,7 +417,7 @@ IPA.batch_command = function (spec) {
 var that = IPA.command(spec);
 
 that.commands = [];
-that.errors = [];
+that.errors = IPA.errors();
 that.error_message = spec.error_message || (IPA.messages.dialogs ?
 IPA.messages.dialogs.batch_error_message : 'Some operations failed.');
 that.show_error = typeof spec.show_error == 'undefined' ?
@@ -434,21 +434,8 @@ IPA.batch_command = function (spec) {
 }
 };
 
-var clear_errors = function() {
-that.errors = [];
-};
-
-var add_error = function(command, name, message, status) {
-that.errors.push({
-command: command,
-name: name,
-message: message,
-status: status
-});
-};
-
 that.execute = function() {
-clear_errors();
+that.errors.clear_errors

Re: [Freeipa-devel] [PATCH] 005 Show error in serial association

2011-08-18 Thread Petr Vobornik

On 08/17/2011 05:38 PM, Petr Vobornik wrote:

Ticket #1628 - https://fedorahosted.org/freeipa/ticket/1628
Unreported insufficient access error

This patch is dependant on
freeipa-pvoborni-0004-1-error-dialog-for-batch-command.patch.

This may be only a checking if approach of this patch is good.

I was not sure if this type of error message (result.failed property) is
more general or it only appears in adding members. So I put error
handling in serial_associator instead of command. If it would be put in
command and success will be transformed to error, it will change the
behaviour of executing commands - other commands after error won't be
executed. If the approach is good, it could be probably better to change
it a little and offer same logic for batch_associator.

It should be working for adding users to groups, netgroups, roles and
assigning hbac rules (tested as non admin user).


Modified association test - data in success handler should not be
undefined.



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

Modified to work with bulk association.

--
Petr Vobornik
From cd0afe270583ce5b90317d7479412a9547048c94 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Wed, 17 Aug 2011 15:06:41 +0200
Subject: [PATCH] Show error in adding associations

https://fedorahosted.org/freeipa/ticket/1628
---
 install/ui/association.js|   47 -
 install/ui/ipa.js|   45 ++-
 install/ui/test/association_tests.js |4 +-
 3 files changed, 73 insertions(+), 23 deletions(-)

diff --git a/install/ui/association.js b/install/ui/association.js
index 2c6a1d2003be0668b61b230b8317263b06a822a5..d8722fac36f781563dfc040b9ecdf0d3a80ed646 100644
--- a/install/ui/association.js
+++ b/install/ui/association.js
@@ -41,9 +41,41 @@ IPA.associator = function (spec) {
 that.on_success = spec.on_success;
 that.on_error = spec.on_error;
 
+that.errors = IPA.errors();
+
 that.execute = function() {
 };
 
+that.check_for_errors = function(command, data, text_status, xhr) {
+var result = data.result;
+if(result  result.failed) {
+for(var association in result.failed) {
+for(var member_name in result.failed[association]) {
+var member = result.failed[association][member_name];
+if(member.length  0  member[0].length  1) {
+var name = 'Association Error';
+var message = member[0][1];
+that.errors.add_error(command, name, message, text_status);
+}
+}
+}
+}
+};
+
+that.show_errors = function () {
+if(that.errors.errors.length  0) {
+var dialog = IPA.error_dialog({
+error_thrown: {
+name: IPA.messages.dialogs.batch_error_title,
+message: IPA.messages.dialogs.batch_error_message
+},
+errors: that.errors.errors,
+visible_buttons: ['ok']
+});
+dialog.open();
+}
+};
+
 return that;
 };
 
@@ -60,12 +92,14 @@ IPA.serial_associator = function(spec) {
 that.execute = function() {
 
 if (!that.values || !that.values.length) {
+that.show_errors();
 that.on_success();
 return;
 }
 
 var value = that.values.shift();
 if (!value) {
+that.show_errors();
 that.on_success();
 return;
 }
@@ -79,7 +113,10 @@ IPA.serial_associator = function(spec) {
 method: that.method,
 args: args,
 options: options,
-on_success: that.execute,
+on_success: function(data, text_status, xhr) {
+that.check_for_errors(command, data, text_status, xhr);
+that.execute();
+},
 on_error: that.on_error
 });
 
@@ -104,12 +141,14 @@ IPA.bulk_associator = function(spec) {
 that.execute = function() {
 
 if (!that.values || !that.values.length) {
+that.show_errors();
 that.on_success();
 return;
 }
 
 var value = that.values.shift();
 if (!value) {
+that.show_errors();
 that.on_success();
 return;
 }
@@ -127,7 +166,11 @@ IPA.bulk_associator = function(spec) {
 method: that.method,
 args: args,
 options: options,
-on_success: that.on_success,
+on_success: function(data, text_status, xhr) {
+that.check_for_errors(command, data, text_status, xhr);
+that.show_errors();
+that.on_success();
+},
 on_error

[Freeipa-devel] [PATCH] 006 Uncheck checkboxes in association after deletion

2011-08-18 Thread Petr Vobornik

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

- added unchecking all checkboxes of table_widget and its derivates to 
on_load method.


line '$('input[name=select]', that.tbody).attr('checked', false);' can 
be unnecessary because load method is recreating all rows in table. But 
I sense, it could be useful later.


As I was implementing it, I noticed that after removing or adding sudo 
option it refreshes facet - all other associations. Don't know if it's 
a feature or a bug. (sudo.js: 679, 683, 723, 727).

--
Petr Vobornik
From b2113a79520dce7e95f5e70678187fcba814f839 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Thu, 18 Aug 2011 13:46:44 +0200
Subject: [PATCH] Uncheck checkboxes in association after deletion

https://fedorahosted.org/freeipa/ticket/1639
---
 install/ui/association.js |1 +
 install/ui/sudo.js|1 +
 install/ui/widget.js  |7 +++
 3 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/install/ui/association.js b/install/ui/association.js
index 2c6a1d2003be0668b61b230b8317263b06a822a5..36adf499980399e68d37e32bbaac56177de39f2d 100644
--- a/install/ui/association.js
+++ b/install/ui/association.js
@@ -461,6 +461,7 @@ IPA.association_table_widget = function (spec) {
 that.load = function(result) {
 that.values = result[that.name] || [];
 that.reset();
+that.unselect_all();
 };
 
 that.update = function() {
diff --git a/install/ui/sudo.js b/install/ui/sudo.js
index 280bec4740ef8b860ab3ba332c8bb7402dc44a97..095480a9a54f94cabf45cb1774ce14e5190468d5 100644
--- a/install/ui/sudo.js
+++ b/install/ui/sudo.js
@@ -1139,6 +1139,7 @@ IPA.sudorule_association_table_widget = function(spec) {
 $.merge(that.values, external_values);
 }
 that.reset();
+that.unselect_all();
 };
 
 return that;
diff --git a/install/ui/widget.js b/install/ui/widget.js
index e30cacd7c2eb86ca4ddf96a65a267a58d156b68f..c01d21649204f8a55f3c97e5ff7b12ffa936605a 100644
--- a/install/ui/widget.js
+++ b/install/ui/widget.js
@@ -1449,6 +1449,12 @@ IPA.table_widget = function (spec) {
 that.select_changed = function() {
 };
 
+that.unselect_all = function() {
+$('input[name=select]', that.thead).attr('checked', false);
+$('input[name=select]', that.tbody).attr('checked', false);
+that.select_changed();
+};
+
 that.empty = function() {
 that.tbody.empty();
 };
@@ -1464,6 +1470,7 @@ IPA.table_widget = function (spec) {
 var record = that.get_record(result, i);
 that.add_record(record);
 }
+that.unselect_all();
 };
 
 that.save = function() {
-- 
1.7.6

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

Re: [Freeipa-devel] [PATCH] 005 Show error in serial association

2011-08-18 Thread Petr Vobornik

On 08/18/2011 10:28 AM, Petr Vobornik wrote:

On 08/17/2011 05:38 PM, Petr Vobornik wrote:

Ticket #1628 - https://fedorahosted.org/freeipa/ticket/1628
Unreported insufficient access error

This patch is dependant on
freeipa-pvoborni-0004-1-error-dialog-for-batch-command.patch.

This may be only a checking if approach of this patch is good.

I was not sure if this type of error message (result.failed property) is
more general or it only appears in adding members. So I put error
handling in serial_associator instead of command. If it would be put in
command and success will be transformed to error, it will change the
behaviour of executing commands - other commands after error won't be
executed. If the approach is good, it could be probably better to change
it a little and offer same logic for batch_associator.

It should be working for adding users to groups, netgroups, roles and
assigning hbac rules (tested as non admin user).


Modified association test - data in success handler should not be
undefined.



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

Modified to work with bulk association.



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


After implementation error notification in associations. I noticed one 
'bug?' :


After adding users to hbac rule, batch error notification is shown 
saying 'no modifications to be performed'.


Reproduce:
- create hbacrule named 'aa'
- add several users - in example 'admin' and 'ttest'

Request:
{method:batch,params:[[{method:hbacrule_mod,params:[[aa],{all:true,rights:true,usercategory:}]},{method:hbacrule_add_user,params:[[aa],{user:admin,ttest}]}],{}]}

Response:

{
error: null,
id: null,
result: {
count: 2,
results: [
{
error: no modifications to be performed
},
{
completed: 2,
error: null,
failed: {
memberuser: {
group: [],
user: []
}
},
result: {
cn: [
aa
],
dn: 
ipauniqueid=cfb492f2-c8dc-11e0-9504-00163e06af05,cn=hbac,dc=vm-021,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com, 


ipaenabledflag: [
TRUE
],
memberuser_group: [
admins
],
memberuser_user: [
admin,
ttest
]
}
}
]
}
}




I think the problem is that the first command should be included only if 
something changed.


It isn't a bug in this patch, but with it it is a new annoyance (you 
have to click OK).


--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 249 Removed 'Hide already enrolled' checkbox.

2011-08-19 Thread Petr Vobornik

On 08/18/2011 04:53 PM, Endi Sukma Dewata wrote:

The 'Hide already enrolled' has been removed from the enrollment
dialog because it is checked by default and entries that are already
enrolled cannot be enrolled again.

Ticket #1638



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
ACK if leaving string in internal.py is the right thing to do when 
deleting related label (the string isn't used anywhere else).


--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 006 Uncheck checkboxes in association after deletion

2011-08-19 Thread Petr Vobornik

On 08/18/2011 10:30 PM, Endi Sukma Dewata wrote:

On 8/18/2011 8:18 AM, Petr Vobornik wrote:

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

- added unchecking all checkboxes of table_widget and its derivates to
on_load method.

line '$('input[name=select]', that.tbody).attr('checked', false);' can
be unnecessary because load method is recreating all rows in table. But
I sense, it could be useful later.

As I was implementing it, I noticed that after removing or adding sudo
option it refreshes facet - all other associations. Don't know if it's
a feature or a bug. (sudo.js: 679, 683, 723, 727).


Some issues:

1. In IPA.table_widget when the select all checkbox is changed the
tooltip will change too (see line 1265). When the checkbox is changed
using unselect_all() it doesn't trigger the 'changed' event so the
tooltip is not updated.

done


I think the code in 1265 can be moved into a separate function and the
unselect_all() can call this function too. You can replace the loop with
your code:
$('input[name=select]', that.tbody).attr('checked', false);

done


2. In IPA.association_facet the select all checkbox is not unchecked
because the content is loaded using refresh_table() in line 1017.

done


3. In IPA.search_facet the select all checkbox is unchecked at line 248.
It would be better to call the table.unselect_all() from the
search_facet.load() at line 226.

done


Optional: the code inside search_facet.load() probably could be moved
into table.load() then the search_facet.load() can call table.load().

done


4. About the sudo options refreshing the whole facet, it would be nicer
if we can limit the update to the options table only. This can be fixed
separately.


agree

--
Petr Vobornik
From 708d424b8c303ed7506043b60d744c98ec4818ec Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Thu, 18 Aug 2011 13:46:44 +0200
Subject: [PATCH] Uncheck checkboxes in association after deletion

https://fedorahosted.org/freeipa/ticket/1639
---
 install/ui/association.js |2 ++
 install/ui/search.js  |   21 -
 install/ui/sudo.js|1 +
 install/ui/widget.js  |   29 -
 4 files changed, 35 insertions(+), 18 deletions(-)

diff --git a/install/ui/association.js b/install/ui/association.js
index 2c6a1d2003be0668b61b230b8317263b06a822a5..3488b5a938207732d35d4c4855a3f1c4a9899650 100644
--- a/install/ui/association.js
+++ b/install/ui/association.js
@@ -461,6 +461,7 @@ IPA.association_table_widget = function (spec) {
 that.load = function(result) {
 that.values = result[that.name] || [];
 that.reset();
+that.unselect_all();
 };
 
 that.update = function() {
@@ -,6 +1112,7 @@ IPA.association_facet = function (spec) {
 that.table.current_page = 1;
 
 that.table.refresh();
+that.table.unselect_all();
 };
 
 that.refresh = function() {
diff --git a/install/ui/search.js b/install/ui/search.js
index bee55c067fe130139b23fe3f32cd15d3daef5457..454ecf45310fadcbc0f758670609be5647cec95f 100644
--- a/install/ui/search.js
+++ b/install/ui/search.js
@@ -86,6 +86,17 @@ IPA.search_facet = function(spec) {
 that.table.refresh = function() {
 that.refresh();
 };
+
+that.table.load = function(result) {
+that.table.empty();
+
+for (var i = 0; iresult.length; i++) {
+var record = that.table.get_record(result[i], 0);
+that.table.add_record(record);
+}
+
+that.table.unselect_all();
+};
 }
 
 that.create_content = function(container) {
@@ -224,13 +235,7 @@ IPA.search_facet = function(spec) {
 };
 
 function load(result) {
-
-that.table.empty();
-
-for (var i = 0; iresult.length; i++) {
-var record = that.table.get_record(result[i], 0);
-that.table.add_record(record);
-}
+that.table.load(result);
 }
 
 that.load = spec.load || load;
@@ -245,8 +250,6 @@ IPA.search_facet = function(spec) {
 
 that.search_refresh = function(entity){
 
-$('input[type=checkbox]',that.table.thead).removeAttr(checked);
-
 function on_success(data, text_status, xhr) {
 
 that.load(data.result.result);
diff --git a/install/ui/sudo.js b/install/ui/sudo.js
index 280bec4740ef8b860ab3ba332c8bb7402dc44a97..095480a9a54f94cabf45cb1774ce14e5190468d5 100644
--- a/install/ui/sudo.js
+++ b/install/ui/sudo.js
@@ -1139,6 +1139,7 @@ IPA.sudorule_association_table_widget = function(spec) {
 $.merge(that.values, external_values);
 }
 that.reset();
+that.unselect_all();
 };
 
 return that;
diff --git a/install/ui/widget.js b/install/ui/widget.js
index e30cacd7c2eb86ca4ddf96a65a267a58d156b68f..f88bba5c2ae2991c0a2d06d571e6b1c9ebc03ab2 100644
--- a/install/ui/widget.js
+++ b/install/ui/widget.js
@@ -1263,16 +1263,11 @@ IPA.table_widget = function (spec

Re: [Freeipa-devel] [PATCH] 005 Show error in serial association

2011-08-22 Thread Petr Vobornik

On 08/18/2011 06:25 PM, Endi Sukma Dewata wrote:

On 8/17/2011 10:38 AM, Petr Vobornik wrote:

Ticket #1628 - https://fedorahosted.org/freeipa/ticket/1628
Unreported insufficient access error

This patch is dependant on
freeipa-pvoborni-0004-1-error-dialog-for-batch-command.patch.

This may be only a checking if approach of this patch is good.

I was not sure if this type of error message (result.failed property) is
more general or it only appears in adding members. So I put error
handling in serial_associator instead of command. If it would be put in
command and success will be transformed to error, it will change the
behaviour of executing commands - other commands after error won't be
executed. If the approach is good, it could be probably better to change
it a little and offer same logic for batch_associator.

It should be working for adding users to groups, netgroups, roles and
assigning hbac rules (tested as non admin user).

Modified association test - data in success handler should not be
undefined.


Currently with serial associator if there's a failure the rest of the
commands will not be executed, so it's an existing problem. You can test
this by adding a user into multiple groups via UI but remove one of the
groups via CLI just before adding. The user will not be added into the
remaining groups. Bulk associator doesn't have this problem because it's
executed as a single command.

I think eventually we want to convert the serial associator to use batch
commands (no need to do it now, but you can if you want). Once it's
converted, all error checking (including result.failed) should be done
in IPA.command so it can be captured by the batch handler.

I'm not sure about other scenarios that will return result.failed, but I
wouldn't assume it's limited to associations. So I think it should be
handled in a more generic way in IPA.command.

One other thing, I think we should avoid using plural class name (i.e.
IPA.errors) because suppose we have a collection of instances of this
class the variable name could become confusing (e.g. that.errorss :) ).
IPA.error_list might be better.



Reworked.

'Failed' moved to command. On 'failed' success is transformed to error - 
can be change behaviour of serial associator in some commands 
(previously some commands were executed even after 'failed' of 
previous). It isn't probably big issue because they fail probably from 
the same reason (consequent commands would fail to).


- 'failed' message is extended by related object (so user would know for 
which command in the batch it is related to).


- there is still the problem ('no modifications to be performed') I 
wrote about on Aug 18. I think it should be fixed before commiting this 
path.


--
Petr Vobornik
From f758ddbcec44c1b2f39d7de84ae216611c03cd43 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Wed, 17 Aug 2011 15:06:41 +0200
Subject: [PATCH] Show error in adding associations

https://fedorahosted.org/freeipa/ticket/1628
---
 install/ui/ipa.js|  100 +++---
 install/ui/test/association_tests.js |4 +-
 2 files changed, 81 insertions(+), 23 deletions(-)

diff --git a/install/ui/ipa.js b/install/ui/ipa.js
index 9a252f1e50fdaf544cb872fe0dbed9377e791559..eebb9f2f14f542f85c35e47c55d51b54f321de62 100644
--- a/install/ui/ipa.js
+++ b/install/ui/ipa.js
@@ -331,6 +331,7 @@ IPA.command = function(spec) {
 }
 
 function success_handler(data, text_status, xhr) {
+var failed;
 
 if (!data) {
 // error_handler() calls IPA.hide_activity_icon()
@@ -347,7 +348,13 @@ IPA.command = function(spec) {
 message: data.error.message,
 data: data
 });
-
+} else if ((failed = that.get_failed(that, data.result, text_status, xhr)) 
+!failed.is_empty()) {
+error_handler.call(this, xhr, text_status,  /* error_thrown */ {
+name: failed.errors[0].name,
+message: failed.errors[0].message,
+data: data
+});
 } else if (that.on_success) {
 IPA.hide_activity_icon();
 //custom success handling, maintaining AJAX call's context
@@ -379,6 +386,25 @@ IPA.command = function(spec) {
 $.ajax(request);
 };
 
+that.get_failed = function(command, result, text_status, xhr) {
+var errors = IPA.error_list();
+if(result  result.failed) {
+for(var association in result.failed) {
+for(var member_name in result.failed[association]) {
+var member = result.failed[association][member_name];
+if(member.length  0  member[0].length  1) {
+var name = 'IPA Error';
+var message = member[0][1];
+if(member[0][0])
+message

Re: [Freeipa-devel] [PATCH] 005 Show error in serial association

2011-08-23 Thread Petr Vobornik

On 08/22/2011 07:05 PM, Endi Sukma Dewata wrote:

On 8/22/2011 10:06 AM, Petr Vobornik wrote:

'Failed' moved to command. On 'failed' success is transformed to error -
can be change behaviour of serial associator in some commands
(previously some commands were executed even after 'failed' of
previous). It isn't probably big issue because they fail probably from
the same reason (consequent commands would fail to).


This will be addressed in ticket #1688.


- 'failed' message is extended by related object (so user would know for
which command in the batch it is related to).


Just to be consistent, I think we should format the message like this:
primary key: error message.


OK

- there is still the problem ('no modifications to be performed') I
wrote about on Aug 18. I think it should be fixed before commiting this
path.


This will be addressed in ticket #1692.

One more issue, a command could return multiple error messages in each
failure, but right now the get_failed() only reads the first message in
each failure. Try adding several users into a group, but remove some of
them just before adding it, only the first missing user is reported. I
think the code in ipa.js:395-401 should iterate through all messages.


Reworked.


I'm thinking that we should show only notification dialog (like in batch 
error for 'failed' commands. The reason is that part of the command can 
be successfully executed, so offering retry is wrong because it may 
cause other error (try it in the example above). Second reason is that 
if we want to show all errors we have to concatenate error messages with 
some separator (quite easy, current implementation) or to pass array of 
error messages  to error dialog  like in batch error (needs to add 
suppor for it in command).


I'm thinking about dialog with this content:

dialog-titleOperation Error/dialog-title

p
Some parts of operation failed. Completed: $completed.
/p
show-hide-link /
ul
li $key: $message /li
li $key2: $message2 /li
/ul
 ok-button

The 'Completed' part would be shown only if present.

Other problem in error dialog is that there are used untranslated 
strings. We should modify it to use translated and as fallback (like in 
init method) untranslated.


--
Petr Vobornik
From 7baa345b59ef61e6539f745bfea5a701cfcff549 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Wed, 17 Aug 2011 15:06:41 +0200
Subject: [PATCH] Show error in adding associations

https://fedorahosted.org/freeipa/ticket/1628
---
 install/ui/ipa.js|  112 +++--
 install/ui/test/association_tests.js |4 +-
 2 files changed, 93 insertions(+), 23 deletions(-)

diff --git a/install/ui/ipa.js b/install/ui/ipa.js
index 9a252f1e50fdaf544cb872fe0dbed9377e791559..c6c8ef2fef9d99642e3695d08811d27587f9fa55 100644
--- a/install/ui/ipa.js
+++ b/install/ui/ipa.js
@@ -331,6 +331,7 @@ IPA.command = function(spec) {
 }
 
 function success_handler(data, text_status, xhr) {
+var failed;
 
 if (!data) {
 // error_handler() calls IPA.hide_activity_icon()
@@ -347,7 +348,18 @@ IPA.command = function(spec) {
 message: data.error.message,
 data: data
 });
+} else if ((failed = that.get_failed(that, data.result, text_status, xhr)) 
+!failed.is_empty()) {
+var message = failed.errors[0].message;
+for(var i = 1; i  failed.errors.length; i++) {
+message += '\n' + failed.errors[i].message;
+}
 
+error_handler.call(this, xhr, text_status,  /* error_thrown */ {
+name: failed.errors[0].name,
+message: message,
+data: data
+});
 } else if (that.on_success) {
 IPA.hide_activity_icon();
 //custom success handling, maintaining AJAX call's context
@@ -379,6 +391,27 @@ IPA.command = function(spec) {
 $.ajax(request);
 };
 
+that.get_failed = function(command, result, text_status, xhr) {
+var errors = IPA.error_list();
+if(result  result.failed) {
+for(var association in result.failed) {
+for(var member_name in result.failed[association]) {
+var member = result.failed[association][member_name];
+for(var i = 0; i  member.length; i++) {
+if(member[i].length  1) {
+var name = 'IPA Error';
+var message = member[i][1];
+if(member[i][0])
+message = member[i][0] + ': ' + message;
+errors.add(command, name, message, text_status);
+}
+}
+}
+}
+}
+return

Re: [Freeipa-devel] [PATCH] 251 Updated add and delete association dialog titles.

2011-08-23 Thread Petr Vobornik

On 08/22/2011 08:50 PM, Endi Sukma Dewata wrote:

The association table widget and facet have been modified to accept
titles for the add and delete dialogs. The table and facet definitions
have been modified to specify the appropriate titles.

Some unused code have been removed.

Ticket #1629



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

ACK

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 252 Removed unnecessary HBAC/sudo rule category modification.

2011-08-23 Thread Petr Vobornik

On 08/22/2011 11:37 PM, Endi Sukma Dewata wrote:

Since the Add/Delete links in the association table are disabled when
the category is set to 'all', it's no longer necessary to check the
category before showing the add/delete dialogs and modify the category
before adding entries. Thus, the IPA.rule_association_table_widget is
no longer needed.

Ticket #1692



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

ACK

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 005 Show error in serial association

2011-08-24 Thread Petr Vobornik

On 08/23/2011 11:09 PM, Endi Sukma Dewata wrote:

On 8/23/2011 6:34 AM, Petr Vobornik wrote:

Please take a look at the attached patch. This should be applied on top
of your patch. It does several things:

1. As mentioned over IRC, we should be treating these partial failure as
a success (the command does return a success). This way it's not going
to show the Retry button.

2. Instead of concatenating the messages, they are now added into the
error list. This way they will appear nicely as a list.

3. If the error dialog appears, it will wait until you click OK before
continuing.

4. Since some of the membership operations are done using serial
associator you might get multiple dialogs, but this should be gone once
we fix #1688.

Please feel free to merge this patch into yours if you want to make
further modifications. Or we can push both patches if you think it's
good enough.

It's good enough for #1628 so we can push both patches.


I can think of some more improvements, but it can be done separately.


I'm not sure if we need to show the completed operations because we
should be able to infer that from the command we're trying to execute
and the error message we're getting. No error means it's completed.

Maybe we should try to provide a better error message, e.g. Some
failures occurred when removing users from group editors.
This isn't probably important. If user is removing some users from group 
he should know that message Some parts of operation failed is related 
to his action.



Also, we
might want to change the 'Operations Error' title because it's actually
a success. How about 'Operation Completed'? This can be done separately.

Agree



If you think showing the completed operations would be useful please
file a ticket and we'll discuss it. We might be able to show the
completed operations under 'Show details'.
It can be useful, but I think it isn't high priority. I filed 
enhancement ticket https://fedorahosted.org/freeipa/ticket/1702 .



Other problem in error dialog is that there are used untranslated
strings. We should modify it to use translated and as fallback (like in
init method) untranslated.


Let's put the locations of any untranslated messages we find into this
ticket: https://fedorahosted.org/freeipa/ticket/1701




--
Petr Vobornik

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


[Freeipa-devel] [PATCH] 007 Validation of details facet before update

2011-08-24 Thread Petr Vobornik

Validation of details facet before update
https://fedorahosted.org/freeipa/ticket/1676 The ticket is a duplicate 
of server error, but it revealed few UI errors.


Newly performs validation of details facet before update. If validation 
fails, notification dialog is shown and command isn't executed.

Fixed integer minimum and maximum value checking.
Read-only and non-writeable fields are no longer considered required.
--
Petr Vobornik
From 8ccfd1ac82eaa2cb322a23166be54836f907c644 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Wed, 24 Aug 2011 15:36:48 +0200
Subject: [PATCH] Validation of details facet before update
 https://fedorahosted.org/freeipa/ticket/1676 The ticket is
 a duplicate of server error, but it revealed few UI errors.

Newly performs validation of details facet before update. If validation fails, notification dialog is shown and command isn't executed.
Fixed integer minimum and maximum value checking.
Read-only and non-writable fields are no longer considered required.
---
 install/ui/details.js  |   31 +--
 install/ui/test/data/ipa_init.json |4 +++-
 install/ui/widget.js   |   12 +++-
 ipalib/plugins/internal.py |6 --
 4 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/install/ui/details.js b/install/ui/details.js
index 564d848d491083ca896eb295129e65b07b64010e..4550c1abfb9d408d332b858638ad28a9811cb0e9 100644
--- a/install/ui/details.js
+++ b/install/ui/details.js
@@ -128,6 +128,18 @@ IPA.details_section = function(spec) {
 return false;
 };
 
+that.is_valid = function() {
+var fields = that.fields.values;
+var valid = true;
+for (var i=0; ifields.length; i++) {
+var field = fields[i];
+if (!field.valid || !field.check_required()) {
+valid = false;
+}
+}
+return valid;
+};
+
 // methods that should be invoked by subclasses
 that.section_create = that.create;
 that.section_setup = that.setup;
@@ -458,12 +470,12 @@ IPA.details_facet = function(spec) {
 on_win(data, text_status, xhr);
 if (data.error)
 return;
-
+
 if (that.post_update_hook) {
 that.post_update_hook(data, text_status);
 return;
 }
-
+
 var result = data.result.result;
 that.load(result);
 }
@@ -488,11 +500,17 @@ IPA.details_facet = function(spec) {
 });
 
 var values;
+var valid = true;
 
 var sections = that.sections.values;
 for (var i=0; isections.length; i++) {
 var section = sections[i];
 
+if(!section.is_valid() || !valid) {
+valid = false;
+continue;
+}
+
 if (section.save) {
 section.save(command.options);
 continue;
@@ -528,6 +546,15 @@ IPA.details_facet = function(spec) {
 }
 }
 
+if(!valid) {
+var dialog = IPA.message_dialog({
+title: IPA.messages.dialogs.validation_title,
+message: IPA.messages.dialogs.validation_message
+});
+dialog.open();
+return;
+}
+
 //alert(JSON.stringify(command.to_json()));
 
 if (that.pre_execute_hook){
diff --git a/install/ui/test/data/ipa_init.json b/install/ui/test/data/ipa_init.json
index 0738df061b50e07847164dd3d049e8615b95f184..9f31a72680852a4586d9637834c04ffbee038b25 100644
--- a/install/ui/test/data/ipa_init.json
+++ b/install/ui/test/data/ipa_init.json
@@ -15907,7 +15907,9 @@
 prospective: Prospective,
 remove_empty: Select entries to be removed.,
 remove_title: Remove ${entity},
-show_details: Show details
+show_details: Show details,
+validation_title: Validation error,
+validation_message: Input form contains invalid or missing values.
 },
 facet_groups: {
 managedby: ${primary_key} is managed by:,
diff --git a/install/ui/widget.js b/install/ui/widget.js
index f88bba5c2ae2991c0a2d06d571e6b1c9ebc03ab2..6ae6f5dfbb549eab24738740625b25c9055e461c 100644
--- a/install/ui/widget.js
+++ b/install/ui/widget.js
@@ -89,7 +89,7 @@ IPA.widget = function(spec) {
 return;
 }
 
-if (meta.minvalue  value  meta.minvalue) {
+if (meta.minvalue !== undefined  value  meta.minvalue) {
 that.valid = false;
 message = IPA.messages.widget.validation.min_value;
 message = message.replace('${value}', meta.minvalue);
@@ -97,7 +97,7 @@ IPA.widget = function(spec) {
 return

Re: [Freeipa-devel] [PATCH] 254 Fixed default map type in automount map adder dialog.

2011-08-25 Thread Petr Vobornik

On 08/24/2011 04:10 AM, Endi Sukma Dewata wrote:

The adder dialog for automount map has been modified to select the
direct map by default.

Ticket #1698



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

ACK

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 256 Fixed host keytab status after setting OTP.

2011-08-26 Thread Petr Vobornik

On 08/26/2011 12:14 AM, Endi Sukma Dewata wrote:

The host details page has been modified to update the keytab status
based on the data returned by the host-mod command for setting OTP.

Ticket #1710



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

ACK

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 009 Modifying sudo options refreshes the whole page

2011-08-29 Thread Petr Vobornik

On 08/26/2011 11:04 PM, Endi Sukma Dewata wrote:

On 8/26/2011 11:41 AM, Petr Vobornik wrote:

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

Currently adding or deleting sudo options will refresh the entire page.
It's not a problem but the code could be optimized to refresh only the
sudo options table


We have several scenarios for sudo options:

1. Add succeeded: The command returns the new record, so we can use it
to load the table. No problem here.

2. Add failed: We may be able to assume the data on the server didn't
change, so we don't have to update the table. (Yes, the old code does a
refresh, but I don't think it's necessary.)
In most cases it's true. One scenario, when update could be useful is 
when there is an network error while receiving response. But in this 
case updating the table would probably ended with the same error. 
Another case is when someone added the same option right before you.


3. Delete batch failed: I think we can assume nothing was executed, same
as #2.

This time only some network issue can occur.


4. Delete batch succeeded: It could contain a mix of successes and
failures. Like you said, we should use the last successful result.

But instead of checking only the last result and do a load() or
update(), we could iterate through the results and find the last
successful one (the one with non-empty result).

Updated


If we find one, then we can use it to load the table. If there isn't
any, it means all failed, so we don't do anything, same as #2.
Same as 2 only for delete operation - you'll end with invalid table in 
concurrent deletion.


What do you think?


Summary:
I would say that the network issue and the same concurrent edit issue 
can be so rare, that the update isn't much necessary and it slows down 
more frequent failures like non-concurrent adding of the same option.


If we want UI to be faster, we should removed updates. If we want it to 
be more valid in rare cases we should keep updates.


Included updated patches for both options (1-without updates, 2-with 
updates).


--
Petr Vobornik
From a3131924fd5513b724864f5661c239fe93a93899 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Fri, 26 Aug 2011 18:36:54 +0200
Subject: [PATCH] Modifying sudo options refreshes the whole page

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

Currently adding or deleting sudo options will refresh the entire page. It's not a problem but the code could be optimized to refresh only the sudo options table
---
 install/ui/sudo.js   |   23 +++
 install/ui/widget.js |   10 +-
 2 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/install/ui/sudo.js b/install/ui/sudo.js
index 8d33550c0d7c25bc8f882084cb8a04662a54ebd7..18dd024f5eec9be21d76218f1e3ba95dc6e7a618 100644
--- a/install/ui/sudo.js
+++ b/install/ui/sudo.js
@@ -574,12 +574,11 @@ IPA.sudo.options_section = function(spec) {
 options: {
 ipasudoopt: value
 },
-on_success: function() {
-that.facet.refresh();
+on_success: function(data) {
+that.load(data.result.result);
 dialog.close();
 },
-on_error: function() {
-that.facet.refresh();
+on_error: function(data) {
 dialog.close();
 }
 });
@@ -618,12 +617,20 @@ IPA.sudo.options_section = function(spec) {
 dialog.execute = function() {
 
 var batch = IPA.batch_command({
-on_success: function() {
-that.facet.refresh();
+on_success: function(data) {
+//last successful result of batch results contains valid data
+var result;
+for(var i = data.result.results.length - 1; i  -1; i--) {
+result = data.result.results[i].result;
+if(result) {
+that.load(result);
+break;
+}
+}
+
 dialog.close();
 },
-on_error: function() {
-that.facet.refresh();
+on_error: function(data) {
 dialog.close();
 }
 });
diff --git a/install/ui/widget.js b/install/ui/widget.js
index 62af6c16d10aac65e51191f2da955b8f1ebb3bed..83cb4dcb23c6a296739bf7e8604ef3f7a6a5b3e7 100644
--- a/install/ui/widget.js
+++ b/install/ui/widget.js
@@ -1471,11 +1471,11 @@ IPA.table_widget = function (spec) {
 that.empty();
 
 that.values = result[that.name];
-if (!that.values) return;
-
-for (var i=0; ithat.values.length; i++) {
-var record = that.get_record(result, i);
-that.add_record(record);
+if (that.values) {
+for (var i=0

Re: [Freeipa-devel] [PATCH] 259 Fixed problem adding hostgroup into netgroup.

2011-08-31 Thread Petr Vobornik

On 08/30/2011 09:06 PM, Endi Sukma Dewata wrote:

The memberof_netgroup association facet for hostgroup has been
explicitly defined to use the serial associator so it will invoke
the right methods.

Ticket #1737



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

ACK

I think, we should unite the usage of serial and bulk associators. 
Currently the usage of entity and other_entity is inverted. This could 
be achieved by proper initialization of spec from association type 
(association name - something similar to what is used now in 
association_table) and do not blindly copy entity from facet as it is 
done now (association entity often isn't equal to facet entity (eg in 
memberof association)). This could be part of 
https://fedorahosted.org/freeipa/ticket/1690 .


--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 260 Fixed problem with combobox.

2011-08-31 Thread Petr Vobornik

On 08/31/2011 07:30 AM, Endi Sukma Dewata wrote:

The entity select widget has been modified to handle timing issue
in both dialog box and details page.

Ticket #1736



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

ACK

--
Petr Vobornik

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


[Freeipa-devel] [PATCH] 011 Attributes table not scrollable

2011-08-31 Thread Petr Vobornik

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

The attributes table (IPA.attributes_widget) used in Permissions, 
Self-Service Permissions, and Delegations is supposed to be short but 
scrollable. In Firefox 3.6 it works fine, but in Firefox 6.0 it appears 
as a long non-scrollable table which makes it more difficult to use.


--
Petr Vobornik
From d54e3d5758771fd010ce60ff0d77ee51964f7bc3 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Wed, 31 Aug 2011 14:42:33 +0200
Subject: [PATCH] Attributes table not scrollable

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

The attributes table (IPA.attributes_widget) used in Permissions, Self-Service Permissions, and Delegations is supposed to be short but scrollable. In Firefox 3.6 it works fine, but in Firefox 6.0 it appears as a long non-scrollable table which makes it more difficult to use.
---
 install/ui/ipa.css |7 ++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/install/ui/ipa.css b/install/ui/ipa.css
index 705e9a11ebf54771669172a85025d17b18aa2ae2..e341b4112bbaa8c8680e8b291aed64d5a3d2e988 100644
--- a/install/ui/ipa.css
+++ b/install/ui/ipa.css
@@ -1035,12 +1035,17 @@ span.main-separator{
 
 }
 
+.aci-attribute-table thead{
+display: block;
+}
+
 
 .aci-attribute-table tbody{
-width: 20em;
+width: 100%;
 height:10em;
 overflow:auto;
 border-bottom: 1px solid #8a8a8a;
+display: block;
 }
 
 .aci-attribute-table th.aci-attribute-column{
-- 
1.7.6

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

[Freeipa-devel] [PATCH] 012 Fixed inconsistency in enabling delete buttons

2011-09-07 Thread Petr Vobornik

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

On the HBAC Rules page, where the rules are listed, if no rule is 
selected, the Delete button is not enabled, and cannot be clicked on.
But edit a Rule, and Delete button is enabled in the available sections 
- regardless of, if an object is selected to be deleted or not, or even 
if there is no object to be selected to delete.


One can click on this button...but then - there is no message indicating 
that something should be selected for deletion for this button to do 
anything.


Milestone: 3.0 Core Effort Backlog
--
Petr Vobornik
From b3a9038b037385a2f037615d83f4e0982d74fbf4 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Wed, 7 Sep 2011 13:57:47 +0200
Subject: [PATCH] Fixed inconsistency in enabling delete buttons

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

On the HBAC Rules page, where the rules are listed, if no rule is selected, the Delete button is not enabled, and cannot be clicked on.
But edit a Rule, and Delete button is enabled in the available sections - regardless of, if an object is selected to be deleted or not, or even if there is no object to be selected to delete.

One can click on this button...but then - there is no message indicating that something should be selected for deletion for this button to do anything.

Notes:
 * fixed association_table_widget and association_facet
---
 install/ui/association.js |   35 +++
 1 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/install/ui/association.js b/install/ui/association.js
index 1c9776b0e6c596be4dd07665b141891d2e7d4ba0..f5559a4fe09f8032f52638f4dde33a7d7980df34 100644
--- a/install/ui/association.js
+++ b/install/ui/association.js
@@ -352,24 +352,25 @@ IPA.association_table_widget = function (spec) {
 
 that.table_create(container);
 
-var remove_button = IPA.action_button({
+that.remove_button = IPA.action_button({
 name: 'remove',
 label: IPA.messages.buttons.remove,
 icon: 'remove-icon',
+'class': 'action-button-disabled',
 click: function() {
-if (!remove_button.hasClass('action-button-disabled')) {
+if (!that.remove_button.hasClass('action-button-disabled')) {
 that.remove_handler();
 }
 return false;
 }
 }).appendTo(that.buttons);
 
-var add_button = IPA.action_button({
+that.add_button = IPA.action_button({
 name: 'add',
 label: IPA.messages.buttons.add,
 icon: 'add-icon',
 click: function() {
-if (!add_button.hasClass('action-button-disabled')) {
+if (!that.add_button.hasClass('action-button-disabled')) {
 that.add_handler();
 }
 return false;
@@ -426,6 +427,19 @@ IPA.association_table_widget = function (spec) {
 }
 };
 
+that.select_changed = function() {
+
+var values = that.get_selected_values();
+
+if (that.remove_button) {
+if (values.length === 0) {
+that.remove_button.addClass('action-button-disabled');
+} else {
+that.remove_button.removeClass('action-button-disabled');
+}
+}
+};
+
 that.get_records = function(on_success, on_error) {
 
 var length = that.values.length;
@@ -829,6 +843,18 @@ IPA.association_facet = function (spec) {
 that.refresh_table();
 };
 
+that.table.select_changed = function() {
+
+var values = that.table.get_selected_values();
+
+if (that.remove_button) {
+if (values.length === 0) {
+that.remove_button.addClass('action-button-disabled');
+} else {
+that.remove_button.removeClass('action-button-disabled');
+}
+}
+};
 }
 
 that.create_header = function(container) {
@@ -842,6 +868,7 @@ IPA.association_facet = function (spec) {
 name: 'remove',
 label: IPA.messages.buttons.remove,
 icon: 'remove-icon',
+'class': 'action-button-disabled',
 click: function() {
 if (!that.remove_button.hasClass('action-button-disabled')) {
 that.show_remove_dialog();
-- 
1.7.6

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

[Freeipa-devel] [PATCH] 013 Fixed: JavaScript type error in entitlement page

2011-09-07 Thread Petr Vobornik

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

Opening IPA Server/Entitlements causes: Uncaught TypeError: Cannot call 
method 'addClass' of undefined error - Details.js:489


--
Petr Vobornik
From 17a3dbed887a75fa1ff0198edb5ae9873b1b35a3 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Wed, 7 Sep 2011 18:19:01 +0200
Subject: [PATCH] Fixed: JavaScript type error in entitlement page

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

Opening IPA Server/Entitlements causes: Uncaught TypeError: Cannot call method 'addClass' of undefined error - Details.js:489

Introduced by patch for #1697

Cause: Details facet of entitlements doesn't contain Reset and Update buttons
---
 install/ui/details.js |   20 ++--
 1 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/install/ui/details.js b/install/ui/details.js
index 4f7e4a29a432f225dc2b652a0e94f98529b84d35..dbdca5a1dae32d3110bec99ee0bc3c2f80db2c0a 100644
--- a/install/ui/details.js
+++ b/install/ui/details.js
@@ -482,12 +482,20 @@ IPA.details_facet = function(spec) {
 };
 
 that.enable_update = function(value) {
-if(value) {
-that.reset_button.removeClass('action-button-disabled');
-that.update_button.removeClass('action-button-disabled');
-} else {
-that.reset_button.addClass('action-button-disabled');
-that.update_button.addClass('action-button-disabled');
+if(that.reset_button) {
+if(value) {
+that.reset_button.removeClass('action-button-disabled');
+} else {
+that.reset_button.addClass('action-button-disabled');
+}
+}
+
+if(that.update_button) {
+if(value) {
+that.update_button.removeClass('action-button-disabled');
+} else {
+that.update_button.addClass('action-button-disabled');
+}
 }
 };
 
-- 
1.7.6

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

Re: [Freeipa-devel] [PATCH] 265 Fixed layout problem in permission adder dialog.

2011-09-08 Thread Petr Vobornik

On 09/08/2011 12:41 AM, Endi Sukma Dewata wrote:

In order to maintain consistent layout between details page and dialog
boxes the IPA.details_list_section has been replaced with
IPA.details_table_section which is based on table.

The IPA.target_section and other subclasses of IPA.details_list_section
have been converted to use IPA.details_table_section as well.

The unit tests have been updated accordingly.

Ticket #1648


Some minor things:

In IPA.details_table_section:
1)not renamed list_section_create method

Code clean-up in aci.js:
2) IPA.rights_section can be deleted  and replaced by spec object usage. 
It doesn't add any functionality.

3) IPA.permission_details_facet can be deleted - it isn't used anywhere.

Should we unite label align? In add dialog labels are aligned left, in 
details table right.



Otherwise it looks OK.

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 265 Fixed layout problem in permission adder dialog.

2011-09-09 Thread Petr Vobornik

On 09/08/2011 06:51 PM, Endi Sukma Dewata wrote:

On 9/8/2011 11:13 AM, Petr Vobornik wrote:

In IPA.details_table_section:
1)not renamed list_section_create method


Fixed.


Code clean-up in aci.js:
2) IPA.rights_section can be deleted and replaced by spec object usage.
It doesn't add any functionality.


Fixed.


3) IPA.permission_details_facet can be deleted - it isn't used anywhere.


Fixed.



ACK

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 266 Fixed sudo rule association dialogs.

2011-09-09 Thread Petr Vobornik

On 09/08/2011 06:53 PM, Endi Sukma Dewata wrote:

On 9/8/2011 10:28 AM, Endi Sukma Dewata wrote:

The adder dialog for the user and host tables in sudo rule details
page have been fixed to use --not-in-sudorules to avoid showing
entries that are already added into the rule either directly or
indirectly via groups.

This does not apply to the command and run-as tables because they
do not support such option.

Ticket #1768


Wrong email title. It should be patch #266.


ACK

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 012 Fixed inconsistency in enabling delete buttons

2011-09-09 Thread Petr Vobornik

On 09/07/2011 09:06 PM, Endi Sukma Dewata wrote:

On 9/7/2011 7:16 AM, Petr Vobornik wrote:

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

One issue, in HBAC/sudo rules details page if the category is changed
from 'all' to 'specific', the Delete button will be enabled although
there is no entries selected.

See the set_enabled() in IPA.association_table_widget. I think if the
parameter is true it should enable only the Add button. If the parameter
is false it disable both Add and Delete button and call unselect_all().


Fixed

--
Petr Vobornik
From ef769ee328dfd0d2293ec3095ddf021eea1fd3ed Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Wed, 7 Sep 2011 13:57:47 +0200
Subject: [PATCH] Fixed inconsistency in enabling delete buttons

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

On the HBAC Rules page, where the rules are listed, if no rule is selected, the Delete button is not enabled, and cannot be clicked on.
But edit a Rule, and Delete button is enabled in the available sections - regardless of, if an object is selected to be deleted or not, or even if there is no object to be selected to delete.

One can click on this button...but then - there is no message indicating that something should be selected for deletion for this button to do anything.

Notes:
 * fixed association_table_widget and association_facet
---
 install/ui/association.js |   40 +++-
 1 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/install/ui/association.js b/install/ui/association.js
index 1c9776b0e6c596be4dd07665b141891d2e7d4ba0..b999f0eade03bc2ab2f2a55d7baadb008ce0d9b7 100644
--- a/install/ui/association.js
+++ b/install/ui/association.js
@@ -352,24 +352,25 @@ IPA.association_table_widget = function (spec) {
 
 that.table_create(container);
 
-var remove_button = IPA.action_button({
+that.remove_button = IPA.action_button({
 name: 'remove',
 label: IPA.messages.buttons.remove,
 icon: 'remove-icon',
+'class': 'action-button-disabled',
 click: function() {
-if (!remove_button.hasClass('action-button-disabled')) {
+if (!that.remove_button.hasClass('action-button-disabled')) {
 that.remove_handler();
 }
 return false;
 }
 }).appendTo(that.buttons);
 
-var add_button = IPA.action_button({
+that.add_button = IPA.action_button({
 name: 'add',
 label: IPA.messages.buttons.add,
 icon: 'add-icon',
 click: function() {
-if (!add_button.hasClass('action-button-disabled')) {
+if (!that.add_button.hasClass('action-button-disabled')) {
 that.add_handler();
 }
 return false;
@@ -420,9 +421,25 @@ IPA.association_table_widget = function (spec) {
 that.set_enabled = function(enabled) {
 that.table_set_enabled(enabled);
 if (enabled) {
-$('.action-button', that.table).removeClass('action-button-disabled');
+if(that.add_button) {
+that.add_button.removeClass('action-button-disabled');
+}
 } else {
 $('.action-button', that.table).addClass('action-button-disabled');
+that.unselect_all();
+}
+};
+
+that.select_changed = function() {
+
+var values = that.get_selected_values();
+
+if (that.remove_button) {
+if (values.length === 0) {
+that.remove_button.addClass('action-button-disabled');
+} else {
+that.remove_button.removeClass('action-button-disabled');
+}
 }
 };
 
@@ -829,6 +846,18 @@ IPA.association_facet = function (spec) {
 that.refresh_table();
 };
 
+that.table.select_changed = function() {
+
+var values = that.table.get_selected_values();
+
+if (that.remove_button) {
+if (values.length === 0) {
+that.remove_button.addClass('action-button-disabled');
+} else {
+that.remove_button.removeClass('action-button-disabled');
+}
+}
+};
 }
 
 that.create_header = function(container) {
@@ -842,6 +871,7 @@ IPA.association_facet = function (spec) {
 name: 'remove',
 label: IPA.messages.buttons.remove,
 icon: 'remove-icon',
+'class': 'action-button-disabled',
 click: function() {
 if (!that.remove_button.hasClass('action-button-disabled')) {
 that.show_remove_dialog();
-- 
1.7.6

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

Re: [Freeipa-devel] [PATCH] 267 Fixed missing optional field.

2011-09-12 Thread Petr Vobornik

On 09/10/2011 02:20 AM, Endi Sukma Dewata wrote:

The optional uid field in user's adder dialog did not appear when
the link is clicked to show the field. This is a regression introduced
in the patch for ticket #1648.

The click handler for the link field has been moved into a new closure
so that the variables point to the correct elements.

Note: the duplicate code in IPA.details_table_section.create() and
IPA.dialog.create() will be addressed separately in ticket #1394.


ACK

But:
The part of code in details.js is never executed because field.optional 
is never set to true for fields in details facet. As you write, some 
clean-up should be addressed in #1394 or/and in #1696 (to be consistent 
with dialogs).


--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 268 Fixed labels for run-as users and groups.

2011-09-12 Thread Petr Vobornik

On 09/10/2011 02:21 AM, Endi Sukma Dewata wrote:

The labels for the run-as users and groups tables in sudo rule details
page have been modified to improve the clarity.

Ticket #1752



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


Use of 'that.label = that.label | ...'  in association_table_widget is 
affecting other labels besides that specified in spec object. In Who 
section of sudo rule label User Group is changed to Groups. Because 
this way it uses default label retrieval method in widget 
(IPA.get_entity_param) instead of metadata.objects.[other_entity].label. 
This isn't entirely wrong, but param labels aren't always consistent 
with entity.labels.


Otherwise its OK.

--
Petr Vobornik

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


[Freeipa-devel] [PATCH] 014 Code cleanup: widget creation

2011-09-13 Thread Petr Vobornik

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

Removed code duplication of undo links.
Simplified code of widget creation to be more readable.
--
Petr Vobornik
From c1e47469cd394c8934e0a6bf3cc84e88b5a6bb5a Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Tue, 13 Sep 2011 13:53:54 +0200
Subject: [PATCH] Code cleanup: widget creation

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

Removed code duplication of undo links.
Simplified code of widget creation to be more readable.
---
 install/ui/aci.js|3 -
 install/ui/widget.js |  182 +++---
 2 files changed, 70 insertions(+), 115 deletions(-)

diff --git a/install/ui/aci.js b/install/ui/aci.js
index a0399f501aaff6cd89e08e03c1de865b03222077..15219e7fef9372e2bbd75b8d8da47e8aa60d4b34 100644
--- a/install/ui/aci.js
+++ b/install/ui/aci.js
@@ -261,9 +261,6 @@ IPA.attributes_widget = function(spec) {
 
 if (that.undo) {
 that.create_undo(container);
-that.get_undo().click(function(){
-that.reset();
-});
 }
 
 if (that.object_type){
diff --git a/install/ui/widget.js b/install/ui/widget.js
index 12a6b966c999d598576d53132c4f37fff685d3b1..58698486894ce9e72842ea1cf011a5fb75286421 100644
--- a/install/ui/widget.js
+++ b/install/ui/widget.js
@@ -287,7 +287,15 @@ IPA.widget = function(spec) {
 return that.dirty;
 };
 
-that.create_undo = function(container) {
+/**
+ * This function creates an undo link in the container.
+ * On_undo is a link click callback. It can be specified to custom
+ * callback. If a callback isn't set, default callback is used. If
+ * spefified to value other than a function, no callback is registered.
+ */
+that.create_undo = function(container, on_undo) {
+container.append(' ');
+
 that.undo_span =
 $('span/', {
 name: 'undo',
@@ -295,6 +303,16 @@ IPA.widget = function(spec) {
 'class': 'ui-state-highlight ui-corner-all undo',
 html: 'undo'
 }).appendTo(container);
+
+if(on_undo === undefined) {
+on_undo = function() {
+that.reset();
+};
+}
+
+if(typeof on_undo === 'function') {
+that.undo_span.click(on_undo);
+}
 };
 
 that.set_dirty = function(dirty) {
@@ -389,61 +407,48 @@ IPA.text_widget = function(spec) {
 IPA.select_range(that.input, start, end);
 };
 
-
 that.create = function(container) {
 
 that.widget_create(container);
 
 container.addClass('text-widget');
 
-$('label/', {
+that.display_control = $('label/', {
 name: that.name,
 style: 'display: none;'
 }).appendTo(container);
 
-$('input/', {
+that.input = $('input/', {
 type: that.type,
 name: that.name,
 disabled: that.disabled,
 size: that.size,
-title: that.tooltip
+title: that.tooltip,
+keyup: function() {
+that.set_dirty(that.test_dirty());
+that.validate();
+}
 }).appendTo(container);
 
 if (that.undo) {
-container.append(' ');
 that.create_undo(container);
 }
 
 that.create_error_link(container);
-
-var input = $('input[name='+that.name+']', that.container);
-input.keyup(function() {
-that.set_dirty(that.test_dirty());
-that.validate();
-});
-
-var undo = that.get_undo();
-undo.click(function() {
-that.reset();
-});
-that.input = input;
 };
 
 that.update = function() {
 var value = that.values  that.values.length ? that.values[0] : '';
 
-var label = $('label[name='+that.name+']', that.container);
-var input = $('input[name='+that.name+']', that.container);
-
 if (that.read_only || !that.writable) {
-label.html(value);
-label.css('display', 'inline');
-input.css('display', 'none');
+that.display_control.html(value);
+that.display_control.css('display', 'inline');
+that.input.css('display', 'none');
 
 } else {
-$('input[name='+that.name+']', that.container).val(value);
-label.css('display', 'none');
-input.css('display', 'inline');
+that.input.val(value);
+that.display_control.css('display', 'none');
+that.input.css('display', 'inline');
 }
 };
 
@@ -452,8 +457,7 @@ IPA.text_widget = function(spec) {
 return null;
 
 } else {
-var input = $('input[name='+that.name+']', that.container);
-var value = input.val();
+var value = that.input.val();
 return value === '' ? [] : [value

Re: [Freeipa-devel] [PATCH] 268 Fixed labels for run-as users and groups.

2011-09-13 Thread Petr Vobornik

On 09/13/2011 08:04 AM, Endi Sukma Dewata wrote:


The labels from entity parameter are actually more appropriate. I've
updated the patch to use them instead. I also fixed some of the labels
(the run-as group label  doc is incorrect).


ACK

--
Petr Vobornik

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


[Freeipa-devel] [PATCH] 015 Fixed: Missing read permission option in RBAC permission

2011-09-13 Thread Petr Vobornik

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

In 'IPA Server/RBAC/Permission/Settings/Rights' is missing a option for 
setting 'read' permission which is supported in CLI.


--
Petr Vobornik
From 6110e275e36adf310fc56d3d72480b1512a76be2 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Tue, 13 Sep 2011 15:05:05 +0200
Subject: [PATCH] Fixed: Missing read permission option in RBAC permission

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

In 'IPA Server/RBAC/Permission/Settings/Rights' is missing a option for setting 'read' permission which is supported in CLI.
---
 install/ui/aci.js |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/install/ui/aci.js b/install/ui/aci.js
index e447391cc7200443562bb5d900594aae77c3beec..bf19ee67b7b4433f699bb54ff301196024a22d0c 100644
--- a/install/ui/aci.js
+++ b/install/ui/aci.js
@@ -381,7 +381,7 @@ IPA.rights_widget = function(spec) {
 
 var that = IPA.checkboxes_widget(spec);
 
-that.rights = ['write', 'add', 'delete'];
+that.rights = ['read', 'write', 'add', 'delete'];
 for (var i=0; ithat.rights.length; i++) {
 var right = that.rights[i];
 that.add_option({label: right, value: right});
-- 
1.7.6

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

Re: [Freeipa-devel] [PATCH] 270 Fixed posix group checkbox.

2011-09-15 Thread Petr Vobornik

On 09/15/2011 02:06 AM, Endi Sukma Dewata wrote:

In the adder dialog for groups the checkbox has been modified to use
the correct field name nonposix and be checked by default.

Note: This is a temporary fix to minimize the changes due to release
schedule. Eventually the field label will be changed into Non-POSIX
group and the checkbox will be unchecked by default, which is more
consistent with CLI.

Ticket #1799


The temporary workaround approach is good but there might be a minor 
issue. One test for this patch fails. It is because current 
implementation sets checked attribute to false for all values except 
'TRUE'. Previous implementation set checked to true if there was any 
value except 'FALSE'. I tried to search for all usages of checkbox and 
found that this behaviour is not a problem right now, but I'm not sure 
if it won't be in the future (but there would have to be changes in save 
method). So if its OK, after a test correction I would consider it ACKed.



--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 273 Removed HBAC rule type.

2011-09-20 Thread Petr Vobornik

On 09/16/2011 06:24 PM, Endi Sukma Dewata wrote:

HBAC rule type has been removed from the list page and details page
because it is no longer supported in IPA 3.0.

Ticket #1795

This should be pushed to master branch only.



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

ACK

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 271 Modified dialog to use sections.

2011-09-22 Thread Petr Vobornik

On 09/21/2011 10:10 PM, Endi Sukma Dewata wrote:
 On 9/21/2011 6:50 AM, Petr Vobornik wrote:

 Fixed. The dialog fields don't need undo, so the text() needs to be
 overridden to disable undo. This can be improved again later.
The override isn't necessary because it wasn't there before and all (at 
least I hope) fields in add dialogs specify undo: false. This feature 
can save some time though. Problem of current implementation is that it 
overrides only the default created section, not the sections specified 
in spec object. But as you wrote - this can be improved later.



 4) host.js:208,217: we should avoid using purely visual inline css
 styles. They should be replaced by class (if cannot be achieved by other
 selector) and styled in css file. This doesn't concern functional styles
 (animations, resizing, hiding, showing).

 Fixed. Yes, we should have a ticket to remove all inline CSS styles.

Are you sure the 'name' attribute is the right way to go? Wouldn't be 
'class' or 'id' (in this case) better? For table data 'name' attribute 
isn't even in HTML spec 
http://dev.w3.org/html5/spec/Overview.html#the-td-element.



--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 279 Fixed problem enrolling member with the same name.

2011-09-22 Thread Petr Vobornik

On 09/20/2011 02:19 AM, Endi Sukma Dewata wrote:

The IPA.association_adder_dialog has been modified to use an exclusion
list to hide entries that are already enrolled.

The IPA.adder_dialog has been modified to store the columns directly
in the available  selected tables.

Ticket #1797



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

ACK

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 281 Fixed problem on combobox with search limit.

2011-09-23 Thread Petr Vobornik

On 09/20/2011 11:18 PM, Endi Sukma Dewata wrote:

The IPA.combobox_widget has been modified such that if the drop-down
list doesn't contain the stored value (due to search limit) it will
not select anything from the list.

The widget has also been modified not to select the value that matches
the filter automatically because that might not be the user's intention.

Ticket #1819


ACK

--
Petr Vobornik

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


[Freeipa-devel] [PATCH] 017 Fixed: Column header for attributes table should be full, width

2011-09-23 Thread Petr Vobornik

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

The column header for the attributes table (IPA.attributes_widget) does 
not cover the entire width of the table. This problem appears in the 
adder dialog and details page for permissions, self-service permissions, 
and delegations.


Note:
 * span element is added in order to set padding for cell text 
because parent th element has css-hacked width. Tbody's use of 
overflow is the cause of used hacks.

--
Petr Vobornik
From 6d5429191f5f530d5aae6b689a2326404ef4f126 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Fri, 23 Sep 2011 17:49:46 +0200
Subject: [PATCH] Fixed: Column header for attributes table should be full
 width

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

The column header for the attributes table (IPA.attributes_widget) does not cover the entire width of the table. This problem appears in the adder dialog and details page for permissions, self-service permissions, and delegations.

Note:
 * span element is added in order to set padding for cell text because parent th element has css-hacked width. Tbody's use of overflow is the cause of used hacks.
---
 install/ui/aci.js  |   17 ++---
 install/ui/ipa.css |   19 ---
 2 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/install/ui/aci.js b/install/ui/aci.js
index 929e13d4b3af3303c32d2122d8f2147afb99b185..868b51ebf7efa4eb3cf67f8acb8fade48e906c05 100644
--- a/install/ui/aci.js
+++ b/install/ui/aci.js
@@ -257,25 +257,28 @@ IPA.attributes_widget = function(spec) {
 var tr = $('tr/tr').appendTo($('thead', that.table));
 
 tr.append($('th/', {
-style:height:2em; vertical-align:bottom;,
-html:$('input/',{
+'class': 'aci-select-column',
+html: $('input/', {
 type: checkbox,
-click: function(){
+click: function() {
 $('.aci-attribute', that.table).
 attr('checked', $(this).attr('checked'));
 that.set_dirty(that.test_dirty());
 }
 })
-})).append($('th/', {
-'class': 'aci-attribute-column',
-html: IPA.messages.objects.aci.attribute
 }));
+var th = $('th/', {
+'class': 'aci-attribute-column'
+}).appendTo(tr);
+$('span/', {
+html: IPA.messages.objects.aci.attribute
+}).appendTo(th);
 
 if (that.undo) {
 that.create_undo(container);
 }
 
-if (that.object_type){
+if (that.object_type) {
 that.populate(that.object_type);
 }
 };
diff --git a/install/ui/ipa.css b/install/ui/ipa.css
index 7876da355edcd8a160c51bdcb1b2db2798192b0f..6d6ab9c84b61b6b0bcc2a7c3400f9bb6ea77d391 100644
--- a/install/ui/ipa.css
+++ b/install/ui/ipa.css
@@ -969,7 +969,9 @@ span.main-separator{
 
 }
 
-.aci-attribute-table thead{
+.aci-attribute-table thead,
+.aci-attribute-table thead tr,
+.aci-attribute-table thead th{
 display: block;
 }
 
@@ -982,12 +984,23 @@ span.main-separator{
 display: block;
 }
 
+.aci-attribute-table th.aci-select-column{
+height:2em;
+float: left;
+}
+
 .aci-attribute-table th.aci-attribute-column{
-width: 20.5em;
+height:2em;
+width: auto;
+line-height: 2em;
+}
+
+.aci-attribute-table th.aci-attribute-column span{
+padding: 0 0.5em;
 }
 
 .entity-views{
- list-style-type:none;
+list-style-type:none;
 }
 
 .entity-views li {
-- 
1.7.6

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

Re: [Freeipa-devel] [PATCH] 017 Fixed: Column header for attributes table should be full, width

2011-09-26 Thread Petr Vobornik

On 09/26/2011 08:15 AM, Endi Sukma Dewata wrote:

On 9/23/2011 11:05 AM, Petr Vobornik wrote:

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

The column header for the attributes table (IPA.attributes_widget) does
not cover the entire width of the table. This problem appears in the
adder dialog and details page for permissions, self-service permissions,
and delegations.


This is an improvement over the existing code, but there's still an
issue. In other tables like in search page there's a space between one
column header to another. In this patch there is no space. It looks like
the header of the 'Attribute' column occupies the entire line (use
Firebug to select the element).


I've reworked the patch. It's simplified and it uses already build-in 
functionality.



For this particular case it's just a minor issue. But suppose we have
more columns it could be a problem because the other headers probably
will occupy the entire line too.

We can push the patch as is if you agree.



I think ideally we want to convert the IPA.attributes_widget to inherit from 
IPA.table_widget. The
table widget should calculate the column widths properly.

I agree.

--
Petr Vobornik
From 12e0dd3addb969b645cb9c45c806da4c612d10c4 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Fri, 23 Sep 2011 17:49:46 +0200
Subject: [PATCH] Fixed: Column header for attributes table should be full
 width

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

The column header for the attributes table (IPA.attributes_widget) does not cover the entire width of the table. This problem appears in the adder dialog and details page for permissions, self-service permissions, and delegations.
---
 install/ui/aci.js  |9 -
 install/ui/ipa.css |   23 +--
 2 files changed, 9 insertions(+), 23 deletions(-)

diff --git a/install/ui/aci.js b/install/ui/aci.js
index 929e13d4b3af3303c32d2122d8f2147afb99b185..fc62f2770979e95768a230c20c80d0aae0e3ce1e 100644
--- a/install/ui/aci.js
+++ b/install/ui/aci.js
@@ -248,7 +248,7 @@ IPA.attributes_widget = function(spec) {
 
 that.table = $('table/', {
 id:id,
-'class':'search-table aci-attribute-table'
+'class':'search-table aci-attribute-table scrollable'
 }).
 append('thead/').
 append('tbody/').
@@ -257,10 +257,9 @@ IPA.attributes_widget = function(spec) {
 var tr = $('tr/tr').appendTo($('thead', that.table));
 
 tr.append($('th/', {
-style:height:2em; vertical-align:bottom;,
-html:$('input/',{
+html: $('input/', {
 type: checkbox,
-click: function(){
+click: function() {
 $('.aci-attribute', that.table).
 attr('checked', $(this).attr('checked'));
 that.set_dirty(that.test_dirty());
@@ -275,7 +274,7 @@ IPA.attributes_widget = function(spec) {
 that.create_undo(container);
 }
 
-if (that.object_type){
+if (that.object_type) {
 that.populate(that.object_type);
 }
 };
diff --git a/install/ui/ipa.css b/install/ui/ipa.css
index 7876da355edcd8a160c51bdcb1b2db2798192b0f..a838195c042ed0e236306e3a15a241c141152a48 100644
--- a/install/ui/ipa.css
+++ b/install/ui/ipa.css
@@ -964,30 +964,17 @@ span.main-separator{
 cursor: default;
 }
 
-
-.aci-attribute-table {
-
-}
-
-.aci-attribute-table thead{
-display: block;
-}
-
-
-.aci-attribute-table tbody{
-width: 100%;
-height:10em;
-overflow:auto;
+.aci-attribute-table tbody {
 border-bottom: 1px solid #8a8a8a;
-display: block;
+height:10em;
 }
 
-.aci-attribute-table th.aci-attribute-column{
-width: 20.5em;
+.aci-attribute-table .aci-attribute-column {
+width: 200em; /* it will fit actual width */
 }
 
 .entity-views{
- list-style-type:none;
+list-style-type:none;
 }
 
 .entity-views li {
-- 
1.7.6

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

Re: [Freeipa-devel] [PATCH] 286 Fixed tab and dialog widths.

2011-09-29 Thread Petr Vobornik

On 09/28/2011 01:02 AM, Endi Sukma Dewata wrote:

The width of the 1st level tab has been modified to expand according
to the size of the tab label.

The width of the adder dialogs have been increased to allow longer
button labels.

Ticket #1825

ACK

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 275 Use editable combobox for service type.

2011-09-29 Thread Petr Vobornik

On 09/28/2011 12:59 AM, Endi Sukma Dewata wrote:

On 9/16/2011 12:16 PM, Endi Sukma Dewata wrote:

The service type field in the service adder dialog has been modified
to use an editable combobox.

Ticket #1633.


Rebased (removed undo parameter).


ACK
--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 288 Disable enroll button if nothing selected.

2011-09-29 Thread Petr Vobornik

On 09/29/2011 06:50 AM, Endi Sukma Dewata wrote:

On 9/28/2011 7:59 PM, Adam Young wrote:

On 09/28/2011 06:50 PM, Endi Sukma Dewata wrote:

A new IPA.dialog_button class has been added to encapsulate the
buttons in the dialog box so they can be managed more easily.

The adder dialog has been modified to disable the enroll button if
there is no entries selected.

Ticket #1856


The solution is simple, but it requires refactoring which probably
should have been done much earlier. So this patch is like a combination
of several patches.




As a further enhancement, the IPA.dialog_button class probably can be
combined with the IPA.action_button/IPA.button class so we can use icons
or enable/disable buttons in a more consistent way. This is one step in
that direction.

I Agree. This would be nice especially in association table widget.

ACK


--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 016 Fixed: Some widgets do not have space for validation error message

2011-09-29 Thread Petr Vobornik

On 09/27/2011 10:54 PM, Endi Sukma Dewata wrote:

On 9/27/2011 7:50 AM, Petr Vobornik wrote:

I've added padding and unified font-weight in error-message style. The
padding is added because text of the message was right on the border
which wasn't much readable. Font-weight is added because sometimes it
inherits font-weight from other style and so it is inconsistent with
other appearances. I'm not sure about the 'bold', though.


This is good but can we use a new CSS class for the error message? I'd
rather not change the jquery-ui.css (removing white spaces is ok)
because it will be difficult to keep track the changes if we need to
upgrade it.
Moved this change to ipa.css (no need to add extra class, it will 
overwrite it as long ipa.css is linked after jquery-ui.css).


There's another minor issue, but this one can be fixed separately if you
want. Try editing a self-service permission, uncheck all attributes,
then click Update. The undo link and the error message appear in 2
separate lines.

Kept them on 2 lines.

I think it's better to show them in a single line. When
we fix the attributes widget to inherit from table, they will fit into
the footer.

I agree.

The padding should be consistent as well, right now the undo
box is smaller than the error message.
Added padding to undo. Added 1px margin-bottom to undo to separate undo 
and error message borders when they are on two lines.


Changed display of undo in attributes_widget from inline to inline-block 
- looks better with added padding.


--
Petr Vobornik
From fdf6af10481885d8e8e2ba012b92129d5a534d17 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Wed, 14 Sep 2011 13:01:25 +0200
Subject: [PATCH] Fixed: Some widgets do not have space for validation error
 message

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

The following widgets should call create_error_link() to create a space to show validation error messages:

  IPA.checkbox_widget
  IPA.checkboxes_widget
  IPA.radio_widget
  IPA.select_widget
  IPA.table_widget
  IPA.attributes_widget
  IPA.rights_widget
  IPA.target_section (it's a widget)

Solution:
 * added call to checkbox, checkboxes, radio, select, table, attributes widget
 * rights_widget inherits it from checkboxes_widget.
 * target_section IS NOT a widget as it doesn't inherit from widget. It's still a section, which shows different widgets based on its state.
 * table_widget displays error_link between pagination and summary.

Additional:
 * added padding and unified font-weight for error message
---
 install/ui/aci.js|6 ++
 install/ui/ipa.css   |   11 ++-
 install/ui/jquery-ui.css |   10 +-
 install/ui/widget.js |   29 -
 4 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/install/ui/aci.js b/install/ui/aci.js
index 676f5df3e63032dd6c6f32279314545608fbcc28..5d5fd8878ca14a5421cf5aa40a3c1384615935e2 100644
--- a/install/ui/aci.js
+++ b/install/ui/aci.js
@@ -315,6 +315,8 @@ IPA.attributes_widget = function(spec) {
 if (that.object_type) {
 that.populate(that.object_type);
 }
+
+that.create_error_link(container);
 };
 
 that.load = function(record) {
@@ -410,6 +412,10 @@ IPA.attributes_widget = function(spec) {
 }
 };
 
+that.show_undo = function() {
+$(that.undo_span).css('display', 'inline-block');
+};
+
 return that;
 };
 
diff --git a/install/ui/ipa.css b/install/ui/ipa.css
index a838195c042ed0e236306e3a15a241c141152a48..20b19eed66fc76490b7a9dde226873a39a83e6e4 100644
--- a/install/ui/ipa.css
+++ b/install/ui/ipa.css
@@ -193,6 +193,13 @@ body {
 padding-right: 0.3em;
 }
 
+.ui-state-error,
+.ui-widget-content .ui-state-error,
+.ui-widget-header .ui-state-error {
+font-weight: bold;
+padding: 0.2em;
+}
+
 /*    Header    */
 #header {
 position: absolute;
@@ -698,12 +705,13 @@ span.main-nav-off  a:visited {
 padding: 0.5em 0 0 1em;
 border-top: 1px solid #dfdfdf;
 height: 25px;
+line-height: 25px;
 margin-top: 1em;
 }
 
 .search-table span[name=summary] {
 float: left;
-line-height: 25px;
+margin-right: 4em;
 }
 
 .search-table span[name=pagination] {
@@ -846,6 +854,7 @@ hr {
 
 .undo {
 cursor:pointer;
+padding: 0.2em;
 }
 
 span.attrhint {
diff --git a/install/ui/jquery-ui.css b/install/ui/jquery-ui.css
index 01c3ec90e2e01c2a6dc677ddf57164c12b9ba0a1..eb0228cd2fdf151872e7de92ff28c8305af8ea2b 100644
--- a/install/ui/jquery-ui.css
+++ b/install/ui/jquery-ui.css
@@ -348,7 +348,7 @@
  *
  * http://docs.jquery.com/UI/Autocomplete#theming
  */
-.ui-autocomplete { position: absolute; cursor: default; }	
+.ui-autocomplete { position: absolute; cursor: default; }
 
 /* workarounds */
 * html .ui-autocomplete { width:1px; } /* without this, the menu expands to 100% in IE6 */
@@ -404,8 +404,8 @@
 .ui-button { display: inline-block; position: relative; padding: 0; margin-right: .1em; text-decoration: none

Re: [Freeipa-devel] [PATCH] 291 I18n clean-up.

2011-10-03 Thread Petr Vobornik

On 10/01/2011 12:09 AM, Endi Sukma Dewata wrote:

The hard-coded 'undo' and 'undo all' labels have been moved into
internal.py to allow translation.

Ticket #1897


ACK

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 292 Disable sudo options Delete button if nothing selected.

2011-10-03 Thread Petr Vobornik

On 10/01/2011 12:10 AM, Endi Sukma Dewata wrote:

The Delete button for sudo options in sudo rule details page now
will only work if there is at least one row selected.

Ticket #1896


ACK


--
Petr Vobornik

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


[Freeipa-devel] [PATCH] 019 Disables gid field if not posix group in group adder dialog

2011-10-04 Thread Petr Vobornik

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

gidNumber is not an allowed attribute for a non-posix group.  When 
adding a non-posix group from the UI, unchecking the Is this a POSIX 
group?: box should disable the GID: field.


--
Petr Vobornik
From 3e329f7f6e26cf839681c95d163625223fb2c546 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Tue, 4 Oct 2011 18:38:08 +0200
Subject: [PATCH] Disables gid field if not posix group in group adder dialog

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

gidNumber is not an allowed attribute for a non-posix group.  When adding a non-posix group from the UI, unchecking the Is this a POSIX group?: box should disable the GID: field.
---
 install/ui/group.js  |   29 -
 install/ui/widget.js |   11 +++
 2 files changed, 39 insertions(+), 1 deletions(-)

diff --git a/install/ui/group.js b/install/ui/group.js
index ad705eb21e4ed06298319110ca4822e86ec701dc..b4753a7ebbeead25ba6e17c8e7e306708606904d 100644
--- a/install/ui/group.js
+++ b/install/ui/group.js
@@ -88,6 +88,7 @@ IPA.entity_factories.group =  function () {
 }).
 standard_association_facets().
 adder_dialog({
+factory: IPA.group_adder_dialog,
 fields: [
 'cn',
 'description',
@@ -115,4 +116,30 @@ IPA.group_nonposix_checkbox_widget = function (spec) {
 };
 
 return that;
-};
\ No newline at end of file
+};
+
+IPA.group_adder_dialog = function (spec) {
+
+spec = spec || {};
+
+var that = IPA.add_dialog(spec);
+
+var init = function() {
+
+var posix_field = that.get_field('nonposix');
+posix_field.value_changed.attach(that.on_posix_change);
+};
+
+that.on_posix_change = function (value) {
+
+var gid_field = that.get_field('gidnumber');
+if(value) {
+gid_field.reset();
+}
+gid_field.set_enabled(!value);
+};
+
+init();
+
+return that;
+};
diff --git a/install/ui/widget.js b/install/ui/widget.js
index f46d79e72309ce367a7f0b3f1fb1f974871ca402..b86f6e04c593f7fbe145f120865e622333092f8e 100644
--- a/install/ui/widget.js
+++ b/install/ui/widget.js
@@ -468,6 +468,15 @@ IPA.text_widget = function(spec) {
 }
 };
 
+that.set_enabled = function(value) {
+
+if(value) {
+that.input.removeAttr('disabled');
+} else {
+that.input.attr('disabled', 'disabled');
+}
+};
+
 // methods that should be invoked by subclasses
 that.text_load = that.load;
 
@@ -771,6 +780,7 @@ IPA.checkbox_widget = function (spec) {
 
 // default value
 that.checked = spec.checked || false;
+that.value_changed = IPA.observer();
 
 that.create = function(container) {
 
@@ -785,6 +795,7 @@ IPA.checkbox_widget = function (spec) {
 title: that.tooltip,
 change: function() {
 that.set_dirty(that.test_dirty());
+that.value_changed.notify(that.save(), that);
 }
 }).appendTo(container);
 
-- 
1.7.6

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

Re: [Freeipa-devel] [PATCH] 019 Disables gid field if not posix group in group adder dialog

2011-10-05 Thread Petr Vobornik

On 10/04/2011 11:59 PM, Adam Young wrote:

On 10/04/2011 12:43 PM, Petr Vobornik wrote:

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

gidNumber is not an allowed attribute for a non-posix group. When
adding a non-posix group from the UI, unchecking the Is this a POSIX
group?: box should disable the GID: field.


Patch would not apply, for some reason. I forced it in by hand. I need
to set up another IPA server to test it, which I will do later on tonight


It applies (git am) on ipa-2-1. With -3 even on master. For completeness 
I attached rebased patch for master.



--
Petr Vobornik
From 975dba79198755dbea7d5037b8819884b1550108 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Tue, 4 Oct 2011 18:38:08 +0200
Subject: [PATCH] Disables gid field if not posix group in group adder dialog

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

gidNumber is not an allowed attribute for a non-posix group.  When adding a non-posix group from the UI, unchecking the Is this a POSIX group?: box should disable the GID: field.
---
 install/ui/group.js  |   29 -
 install/ui/widget.js |   11 +++
 2 files changed, 39 insertions(+), 1 deletions(-)

diff --git a/install/ui/group.js b/install/ui/group.js
index b9664ea6141736e7a2415d2548f838ecd15de1fd..a63a7800a0b668152188bfccfd372e8548e484fd 100644
--- a/install/ui/group.js
+++ b/install/ui/group.js
@@ -102,6 +102,7 @@ IPA.entity_factories.group =  function () {
 }).
 standard_association_facets().
 adder_dialog({
+factory: IPA.group_adder_dialog,
 fields: [
 'cn',
 {
@@ -133,4 +134,30 @@ IPA.group_nonposix_checkbox_widget = function (spec) {
 };
 
 return that;
-};
\ No newline at end of file
+};
+
+IPA.group_adder_dialog = function (spec) {
+
+spec = spec || {};
+
+var that = IPA.add_dialog(spec);
+
+var init = function() {
+
+var posix_field = that.get_field('nonposix');
+posix_field.value_changed.attach(that.on_posix_change);
+};
+
+that.on_posix_change = function (value) {
+
+var gid_field = that.get_field('gidnumber');
+if(value) {
+gid_field.reset();
+}
+gid_field.set_enabled(!value);
+};
+
+init();
+
+return that;
+};
diff --git a/install/ui/widget.js b/install/ui/widget.js
index b25dc8f7f085dedb839d37631509621290008610..d869b57edffeac3078e8b054f30994cec377f5a4 100644
--- a/install/ui/widget.js
+++ b/install/ui/widget.js
@@ -468,6 +468,15 @@ IPA.text_widget = function(spec) {
 }
 };
 
+that.set_enabled = function(value) {
+
+if(value) {
+that.input.removeAttr('disabled');
+} else {
+that.input.attr('disabled', 'disabled');
+}
+};
+
 // methods that should be invoked by subclasses
 that.text_load = that.load;
 
@@ -771,6 +780,7 @@ IPA.checkbox_widget = function (spec) {
 
 // default value
 that.checked = spec.checked || false;
+that.value_changed = IPA.observer();
 
 that.create = function(container) {
 
@@ -785,6 +795,7 @@ IPA.checkbox_widget = function (spec) {
 title: that.tooltip,
 change: function() {
 that.set_dirty(that.test_dirty());
+that.value_changed.notify(that.save(), that);
 }
 }).appendTo(container);
 
-- 
1.7.6

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

[Freeipa-devel] [PATCH] 023 Circular entity dependency

2011-10-10 Thread Petr Vobornik

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

(3.0 Core Effort Iteration 01 September Y11 Release)

Implemented solution:
 * all entities are created on application start
 * dependant objects (facets and dialogs) are created at once on their 
first use in entity.


Note(patch naming): patch 022 was second part of 021, but the file name 
was wrong(021-1)

--
Petr Vobornik
From 4785e6bbc9d4254025bb3e06e865f2eccac9f36f Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Mon, 10 Oct 2011 13:34:15 +0200
Subject: [PATCH] Circular entity dependency

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

Each entity is created together with its dependent objects (e.g. facets and dialog boxes). This causes a circular dependency problem because some of the objects need to obtain a reference to another entity that has not been created.

Currently this is handled by storing only the other entity name and resolve it when needed (e.g. during rendering stage). In IPA.search_facet this delays the creation of the table widget, making it more difficult to customize.

One solution is to do the object creation in 2 steps:

 * create all entity objects only
 * create the dependent objects in each entity

Implemented solution:
 * all entities are created on application start
 * dependant objects (facets and dialogs) are created at once on their first use in entity.
---
 install/ui/details.js|   23 
 install/ui/entity.js |  308 +++--
 install/ui/navigation.js |6 +-
 install/ui/search.js |4 +-
 4 files changed, 240 insertions(+), 101 deletions(-)

diff --git a/install/ui/details.js b/install/ui/details.js
index e25c45875bcf4950e0c3085fd3930aea764d35bb..1e4a9eb5f1148cb109a89d0affd6aee3c4c6eefd 100644
--- a/install/ui/details.js
+++ b/install/ui/details.js
@@ -295,6 +295,27 @@ IPA.details_facet = function(spec) {
 
 that.sections = $.ordered_map();
 
+that.add_sections = function(sections) {
+
+if(sections) {
+for(var i=0; i  sections.length; i++) {
+
+var section_spec = sections[i];
+section_spec.entity = that.entity;
+
+if (!section_spec.label) {
+var obj_messages = IPA.messages.objects[that.entity.name];
+section_spec.label = obj_messages[section_spec.name];
+}
+
+section_spec.factory = section_spec.factory || IPA.details_table_section;
+var section = section_spec.factory(section_spec);
+
+that.add_section(section);
+}
+}
+};
+
 that.dirty = false;
 
 that.add_section = function(section) {
@@ -717,6 +738,8 @@ IPA.details_facet = function(spec) {
 command.execute();
 };
 
+that.add_sections(spec.sections);
+
 that.details_facet_create_content = that.create_content;
 that.details_facet_load = that.load;
 
diff --git a/install/ui/entity.js b/install/ui/entity.js
index 3742360115d9dda51f35132b648d785a600d38f3..7545c8a2ba66aecde8da07bf1f5248d2aba08b55 100644
--- a/install/ui/entity.js
+++ b/install/ui/entity.js
@@ -128,7 +128,7 @@ IPA.facet = function (spec) {
 that.redirect = function() {
 var entity = that.entity;
 while (entity.containing_entity) {
-entity = entity.containing_entity;
+entity = entity.get_containing_entity();
 }
 
 IPA.nav.show_page(
@@ -192,13 +192,13 @@ IPA.facet_header = function(spec) {
 
 if (!that.facet.disable_breadcrumb) {
 var breadcrumb = [];
-var entity = that.facet.entity.containing_entity;
+var entity = that.facet.entity.get_containing_entity();
 
 while (entity) {
 breadcrumb.unshift($('a/', {
 'class': 'breadcrumb-element',
 text: IPA.nav.get_state(entity.name+'-pkey'),
-title: entity.name,
+title: entity.title,
 click: function(entity) {
 return function() {
 IPA.nav.show_page(entity.name, 'default');
@@ -207,7 +207,7 @@ IPA.facet_header = function(spec) {
 }(entity)
 }));
 
-entity = entity.containing_entity;
+entity = entity.get_containing_entity();
 }
 
 that.path.empty();
@@ -293,7 +293,9 @@ IPA.facet_header = function(spec) {
 }).appendTo(that.breadcrumb);
 
 var entity = that.facet.entity;
-while (entity.containing_entity) entity = entity.containing_entity;
+while (entity.containing_entity) {
+entity = entity.get_containing_entity();
+}
 
 $('a/', {
 text: entity.metadata.label,
@@ -453,9 +455,13 @@ IPA.entity = function (spec) {
 that.title = spec.title || that.label

[Freeipa-devel] [PATCH] 024 Added missing fields to password policy page

2011-10-11 Thread Petr Vobornik

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

(2.1.3 Release)

No editable fields exist for maxfail, failinterval lockouttime and 
priority in password policy page.


--
Petr Vobornik
From 9ae5eca65de34c02fe0c3baae6eb27e2fa8fe346 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Tue, 11 Oct 2011 10:24:48 +0200
Subject: [PATCH] Added missing fields to password policy page

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

No editable fields exist for maxfail, failinterval lockouttime and priority in password policy page.
---
 install/ui/policy.js |   12 ++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/install/ui/policy.js b/install/ui/policy.js
index ac9eb20f71d0e6765aaa069fbae27304018511a4..af9e3b85952ec26c36c399e6d5f1b301c34c15a2 100644
--- a/install/ui/policy.js
+++ b/install/ui/policy.js
@@ -39,8 +39,16 @@ IPA.entity_factories.pwpolicy = function() {
 name: 'cn',
 other_entity: 'group'
 },
-'krbmaxpwdlife','krbminpwdlife','krbpwdhistorylength',
-'krbpwdmindiffchars','krbpwdminlength']
+'krbmaxpwdlife',
+'krbminpwdlife',
+'krbpwdhistorylength',
+'krbpwdmindiffchars',
+'krbpwdminlength',
+'krbpwdmaxfailure',
+'krbpwdfailurecountinterval',
+'krbpwdlockoutduration',
+'cospriority'
+]
 }]}).
 standard_association_facets().
 adder_dialog({
-- 
1.7.6

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

[Freeipa-devel] [PATCH] 026 Fixed: Unable to add external user for RunAs User for Sudo

2011-10-17 Thread Petr Vobornik

https://fedorahosted.org/freeipa/ticket/1987
--
Petr Vobornik
From 931b27dbb54ace65e2213ffed718ee04ace5fc07 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Mon, 17 Oct 2011 11:48:03 +0200
Subject: [PATCH] Fixed: Unable to add external user for RunAs User for Sudo
 rules

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

There is no way to add root or any external user as a RunAs User for a Sudo
Rule.
---
 install/ui/sudo.js |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/install/ui/sudo.js b/install/ui/sudo.js
index 8228951c30a71c1e0fb93f0477fa7e07079b27ba..af625661edc799f7dedeeaae44aed8e281e7ebf9 100644
--- a/install/ui/sudo.js
+++ b/install/ui/sudo.js
@@ -912,6 +912,7 @@ IPA.sudo.rule_details_runas_section = function(spec) {
 id: that.entity.name+'-runasruser_user',
 name: 'ipasudorunas_user',
 entity: that.entity,
+external: 'ipasudorunasextuser',
 add_method: 'add_runasuser',
 remove_method: 'remove_runasuser',
 add_title: IPA.messages.association.add.ipasudorunas,
@@ -946,6 +947,7 @@ IPA.sudo.rule_details_runas_section = function(spec) {
 id: that.entity.name+'-runasgroup_group',
 name: 'ipasudorunasgroup_group',
 entity: that.entity,
+external: 'ipasudorunasextgroup',
 add_method: 'add_runasgroup',
 remove_method: 'remove_runasgroup',
 add_title: IPA.messages.association.add.ipasudorunasgroup,
-- 
1.7.6.4

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

Re: [Freeipa-devel] [PATCH] 023 Circular entity dependency

2011-10-18 Thread Petr Vobornik

Comments in text. Also attached git diff for convenience.

On 10/17/2011 10:39 PM, Endi Sukma Dewata wrote:
 On 10/10/2011 10:13 AM, Petr Vobornik wrote:
 https://fedorahosted.org/freeipa/ticket/1531

 (3.0 Core Effort Iteration 01 September Y11 Release)

 Implemented solution:
 * all entities are created on application start
 * dependant objects (facets and dialogs) are created at once on their
 first use in entity.

 Note(patch naming): patch 022 was second part of 021, but the file name
 was wrong(021-1)

 Some comments/issues:

 1. One of the goals of this bug is to remove the temporary workaround in
 IPA.search_facet.create_content(). We should now be able to call the
 initialize_table_columns() during facet initialization.

Fixed

 2. Using lazy-loading to create entities, facets, and dialogs makes
 object creations a little bit unpredictable. This is probably fine for
 now, but if there's a problem the other option is to create all objects
 during application initialization. We can use a loop to create all
 entities first, then use another loop to create all dependent objects in
 each entity.

I don't think it's necessary  but if it becomes a problem, we can use 
the initialization loop.


 3. Another goal is to replace entity names used in spec (see
 other_entity  nested_entity spec properties) with the actual entity
 objects. In this case it might be better to use the loops described in
 #2. This can be done separately.

Wouldn't it lead to the circular dependancy problem again? I think using 
entity names and calling IPA.get_entity at the time when it is needed is 
fine. But we should make some naming conversions of function params or 
object properties to distinguish when we are working with just name or 
entity itself.


 4. In the original code, when creating a facet for indirect association
 it will try to find the corresponding direct facet and use it instead of
 creating a new one. In the new code, the indirect facet will always be
 created, but since there is no indirect facet group the facet will never
 appear. It would be better if we can avoid unnecessary creation of
 indirect facets.

Fixed

 5. In entity.js:201, the use of entity.title for the breadcrumb tooltip
 might not be appropriate because usually the title is plural whereas the
 breadcrumb points to a single object. It would be better to use the
 entity.metadata.label_singular.

Fixed

 6. Invoking a method by concatenating the method name dynamically such
 as prepare_facet type_spec will work, but it's more error prone and
 will clutter up the namespace. It would be better to store the methods
 in a map like this:

 that.map.put('search', function(spec) {
 ...
 });

 and use it like this:

 var method = that.map.get('search');
 method(spec);

 This can be done separately.
Reworked. Used object as a map, ordered map isn't necessary.


 7. The code in entity.js:474,998,1000 should have a deeper indentation
 because it's a continuation of the previous line.

Fixed

 8. The facet_specs and dialog_specs lists can be replaced with
 ordered_map. It already has a method to find an element by its name.
 This can be done separately.

My intention was to be able specify entity facets and dialogs simply by 
specifying their spec in entity spec (without calling builder function), 
this is possible now. I didn't want to add initialization logic for 
facet_specs (to fill the ordered_map) to entity in order to keep it as 
simple as possible. It would be nice though. Maybe we could make an 
utility object with methods for some simple initialization logic which 
is used on many places - like checking if boolean is set or some more 
complicated like initialization of ordered_map (needs key selector fn as 
parameter).


--
Petr Vobornik
From ac6770e841f1af7476e1b3b34e3d7bbcb1fc77b6 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Mon, 10 Oct 2011 13:34:15 +0200
Subject: [PATCH] Circular entity dependency

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

Each entity is created together with its dependent objects (e.g. facets and dialog boxes). This causes a circular dependency problem because some of the objects need to obtain a reference to another entity that has not been created.

Currently this is handled by storing only the other entity name and resolve it when needed (e.g. during rendering stage). In IPA.search_facet this delays the creation of the table widget, making it more difficult to customize.

One solution is to do the object creation in 2 steps:

 * create all entity objects only
 * create the dependent objects in each entity

Implemented solution:
 * all entities are created on application start
 * dependant objects (facets and dialogs) are created at once on their first use in entity.
---
 install/ui/details.js|   23 +++
 install/ui/entity.js |  340 +-
 install/ui/navigation.js |6 +-
 install/ui/search.js |   10 +-
 4 files changed, 275 insertions

Re: [Freeipa-devel] [PATCH] 023 Circular entity dependency

2011-10-20 Thread Petr Vobornik

Fix for regression in unit test, introduced by previous patch.

--
Petr Vobornik
From f1a1a99763f38a10304c374da88857c85c9e7748 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Thu, 20 Oct 2011 16:37:48 +0200
Subject: [PATCH] Fixing infinite loop in UI navigation unit test.

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

It's a fix for regression introduced by previous patch.
---
 install/ui/test/navigation_tests.js |   12 +---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/install/ui/test/navigation_tests.js b/install/ui/test/navigation_tests.js
index 90e708ccb3daeb63031c76884bd69b608fbf5694..6df1926c3efac17c0eada193e00a6dcebf0d7093 100644
--- a/install/ui/test/navigation_tests.js
+++ b/install/ui/test/navigation_tests.js
@@ -45,9 +45,15 @@ test(Testing IPA.navigation.create()., function() {
 IPA.entities = $.ordered_map();
 
 IPA.entity_factories.user =  function() {
-var that = IPA.entity({name: 'user',
-   metadata:IPA.metadata.objects.user});
-that.add_facet(IPA.search_facet({'entity':that}));
+var that = IPA.entity({
+name: 'user',
+metadata:IPA.metadata.objects.user,
+facets: [
+{
+type: 'search'
+}
+]
+});
 
 that.display = function(container){
 user_mock_called = true;
-- 
1.7.6.4

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

Re: [Freeipa-devel] [PATCH] 295 Fixed inconsistent required/optional attributes.

2011-10-21 Thread Petr Vobornik

On 10/20/2011 09:35 PM, Endi Sukma Dewata wrote:

The dialogs and details pages have been modified to use the * symbol
to mark required fields. The automount map and the DNS zone dialogs
have been modified to update the required fields according to the
input type.

Ticket #1696



1) Wouldn't be better if the asterisk has different color than the 
label? Visually I don't like it that much and I think it can be 
overlook. Attaching a proposition. I used green IPAish color because red 
usually means error.


Code from browser how it was done:

td class=section-cell-label title=First namelabel 
name=givenname class=field-labelFirst name:/labelspan 
class=required style=

float:  right;
font-weight: bold;
color: #319016;
font-size: 20px;
 title=required*/span/td

(style should be moved to css file)


div style=line-height: 25px;span class=required style=
font-weight: bold;
color: #319016;
font-size: 20px;
vertical-align: middle;
*/span required/div

It may vary on the section type.

2) When rendering label, we should also obtain field input's id (if 
possible) for 'for' attribute of label. This can be done separately.


3) Should we create some common pure html widgets with certain 
semantics? IE asterisk shouldn't be directly concatenated with label 
text. It is used on more than one place which may cause maintenance issues.


IPA.form(or some other name).required_indicator = function() {
return '*'  
};

in this case this seems unnecessary. But if the required indicator was 
like in 1) it will be useful.



Summary:
All 3 points are nice to have. If you think is not necessary then ACK.

This patch is also fixing https://fedorahosted.org/freeipa/ticket/1973 .

--
Petr Vobornik
attachment: required-field.png___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 028 Code cleanup of HBAC, Sudo rules

2011-10-25 Thread Petr Vobornik
Comments in text. New patch not supplied yet - some topics may require 
further discussion. Most of the comments should be part of the 'Nesting 
widgets' thread.


On 10/24/2011 06:29 PM, Endi Sukma Dewata wrote:

On 10/24/2011 3:29 AM, Petr Vobornik wrote:

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


Some issues:

1. The IPA.hbacrule_details_facet uses IPA.sudo.enable_widget(). This
makes HBAC unnecessarily dependent on sudo. The enable_widget should be
moved to widget.js or rule.js which is shared between HBAC and sudo.


 * enable_widget moved to widget.js
 * rule_association_table_widget and rule_association_adder_dialog 
moved to rule.js



2. The set_facet() is added to widget. I don't think we want to make
widget dependent on facet. The facet so far is only used by
IPA.sudo.enable_widget. In IPA.sudo.options_section the facet object is
passed as a parameter in spec.
Are you saying that dependency on facet in ALL widgets is bad and only 
in a few is good, or in any is bad?


I think it doesn't matter if all is dependant when one is dependant.

Anyway currently all association table widgets are dependant 
(association.js:386).


Btw similar topic could be: How to get current entity's pkey?'. 
Dependancy on facet.get_primary_key() or 
IPA.nav.get_state(that.entity.name+'-pkey') seems wrong too.




3. When updating sudo rule, in the original code the rule is
enabled/disabled after all the modifications are done. In the new code
it's reversed. I'm not sure the importance of the execution order for
this particular situation, but suppose it is, is there a way to maintain
the original order?


I was thinking about a command or widget priority. The 'mod' command 
would have some default priority or it would get it from facet. The 
commands would be sorted by priority before adding them to batch. This 
way we can set exact order in facet update.


The order which is implemented now is reflecting the order in HBAC 
details facet - there were errors when 'mod' was executed before 
clearing associations.



4. The IPA.header_widget is not really a widget, it's actually part of
layout. In the current implementation a widget always corresponds to an
attribute, so it will be loaded, saved, etc. Since there is no attribute
name matching the header name currently this is not a problem, but it
can pollute the namespace.


This is part of widget nesting topic. In this manner composite_widget 
isn't a proper widget too (it can correspond to several or none attribute).


Purely html rendering widgets can be separated from attribute widgets, 
but it would be nice to have them because they can provide easier form 
composing. Without them it would be required to override the create 
method (in facet or composite_widget/section) which is sometimes better 
but often it creates only code duplication with little added logic.




5. In details facet update(), instead of storing the callbacks in
update_custom_on_win and update_custom_on_fail and requiring the
subclasses to call them, it might be better to call them from the
update() itself:

var command = IPA.command({
...
on_success: function(data, text_status, xhr) {
that.on_update_success(data, text_status, xhr)
if (on_win) on_win.call(this, data, text_status, xhr);
},
on_error: function(xhr, text_status, error_thrown) {
that.on_update_error(xhr, text_status, error_thrown);
if (on_fail) on_fail.call(
this, xhr, text_status, error_thrown);
}
});


There are two types of success/error handlers:
1) the default ones in details facet
2) the ones supplied to update(win, fail) method.

In my implementation I have separated 1) from update method so if 
default behaviour isn't suitable these methods can be overridden without 
overriding whole update method. Overriding isn't required. This is IMHO 
good (less code duplication).





6. Can IPA.batch_details_facet be combined into IPA.details_facet? I
think the base details facet should support both regular mod and batch.


The only point where they overlap is the update method. It's possible to 
add a logic to switch between command creations - maybe based on 
attribute (could be defined in spec). But should we do that? Isn't this 
the reason for inheriting the class?




7. In sudo details page, the As Whom category is missing the RunAs
User category radio buttons.


Fixed


8. The IPA.sudo.rule_details_runas_widget uses composite widget instead
of section. They are right now functionally identical, but I suppose the
composite widget is an abstract class and the section will later contain
code specific to section.


Part of widget nesting topic. This was briefly discussed with Adam (in 
that topic). The important question is what is a section? I understand 
section as top level container (in facet) which contains widgets (even 
composites) and semantically separates parts of the facet (with headers 
and section folding...). From this point of view, composite_widget isn't 
an abstract class, but it isn't used used anywhere else

Re: [Freeipa-devel] [PATCH] 298 Fixed host Enrolled column.

2011-10-26 Thread Petr Vobornik

On 10/25/2011 10:12 PM, Endi Sukma Dewata wrote:

The Enrolled column in the host search page has been added back
to show the host enrollment status based on has_keytab attribute.

Ticket #2020



UI directly shows the value (true/false) of has_keytab in the column. 
This is wrong when using different language. It should be translated. We 
can fix it separately, it is also present in SUDO rules search facet.


--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 297 Fixed enroll labels.

2011-10-27 Thread Petr Vobornik

On 10/26/2011 07:44 PM, Endi Sukma Dewata wrote:

On 10/26/2011 7:09 AM, Petr Vobornik wrote:

1) add_dialog was renamed to entity_adder_dialog but its method
add_dialog_create wasn't renamed


Fixed.


ACK

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 300 Refactored validation code.

2011-10-27 Thread Petr Vobornik

On 10/26/2011 08:09 PM, Endi Sukma Dewata wrote:

The validation code in details facet, dialog, and sections have
been modified to work more consistently.

This is required by patch #301.



ACK

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 301 Added password field in user adder dialog.

2011-10-27 Thread Petr Vobornik

On 10/26/2011 08:11 PM, Endi Sukma Dewata wrote:

The user adder dialog has been modified to provide optional fields
to specify password during user creation.

Ticket #1646



1)  message in: 'field2.show_error('Passwords do not match');' should 
use translated string.


--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 302 Fixed inconsistent image names.

2011-10-27 Thread Petr Vobornik

On 10/27/2011 12:31 AM, Endi Sukma Dewata wrote:

The images have been renamed to be more consistent and moved into
the images directory to mimic the original jQuery UI structure.

Ticket #1613


ACK

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 303 Fixed inconsistent details facet validation.

2011-10-27 Thread Petr Vobornik

On 10/27/2011 01:39 AM, Endi Sukma Dewata wrote:

The details facet validation has been moved out of update() such
that all subclasses perform consistent validation.

Ticket #1455



ACK with a small doubt.

I'm not sure if moving the validation call to update button's click 
handler is the right move. I think, this way the handler is doing more 
things than it should do. However I'm fond of changing the update method 
to be more override friendly.


--
Petr Vobornik

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


[Freeipa-devel] [PATCH] 029 Page is cleared before it is visible

2011-10-27 Thread Petr Vobornik

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

Changes:
 * added clear method to widgets, section, search, details, association 
facets

 * clear method in facet is called only if key/filter was changed
--
Petr Vobornik
From 22d6ba37f74ec40a8223082b8f6869ec9f1155a5 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Mon, 24 Oct 2011 14:53:29 +0200
Subject: [PATCH] Page is cleared before it is visible

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

Changes:
 * added clear method to widgets, section, search, details, association facets
 * clear method in facet is called only if key/filter was changed
---
 install/ui/association.js |   11 ++
 install/ui/certificate.js |9 
 install/ui/details.js |   23 +
 install/ui/host.js|   12 +++
 install/ui/search.js  |   11 ++
 install/ui/service.js |5 
 install/ui/user.js|5 
 install/ui/widget.js  |   48 +
 8 files changed, 124 insertions(+), 0 deletions(-)

diff --git a/install/ui/association.js b/install/ui/association.js
index ebb6e421ff3b8538116471de240b1f972e08e6bf..f7e397c92fb97e0a3b1552f29bb3af9da6a55756 100644
--- a/install/ui/association.js
+++ b/install/ui/association.js
@@ -1198,9 +1198,20 @@ IPA.association_facet = function (spec) {
 
 command.on_error = that.on_error;
 
+var old_pkey = that.old_pkey;
+that.old_pkey = pkey;
+
+if (!old_pkey || old_pkey[0] !== pkey[0]) {
+that.clear();
+}
+
 command.execute();
 };
 
+that.clear = function() {
+that.table.clear();
+};
+
 /*initialization*/
 var adder_columns = spec.adder_columns || [];
 for (var i=0; iadder_columns.length; i++) {
diff --git a/install/ui/certificate.js b/install/ui/certificate.js
index 6136edaf0bbcedac890c8c8a6df3297d6802ccc9..70fc1ba3545a5339f873f47cc7656a0953fb50fd 100755
--- a/install/ui/certificate.js
+++ b/install/ui/certificate.js
@@ -725,6 +725,15 @@ IPA.cert.status_widget = function(spec) {
 }
 };
 
+that.clear = function() {
+that.status_valid.css('display', 'none');
+that.status_missing.css('display', 'none');
+that.status_revoked.css('display', 'none');
+that.revoke_button.css('display', 'none');
+that.restore_button.css('display', 'none');
+that.revocation_reason.text('');
+};
+
 function set_status(status, revocation_reason) {
 that.status_valid.css('display', status == IPA.cert.CERTIFICATE_STATUS_VALID ? 'inline' : 'none');
 that.status_missing.css('display', status == IPA.cert.CERTIFICATE_STATUS_MISSING ? 'inline' : 'none');
diff --git a/install/ui/details.js b/install/ui/details.js
index 5c03de0a32aed46aaebd36facddceaf56a853004..a863c18289c2df45aaecec0a68f464c5dc591bf8 100644
--- a/install/ui/details.js
+++ b/install/ui/details.js
@@ -201,6 +201,14 @@ IPA.details_section = function(spec) {
 }
 };
 
+that.clear = function() {
+var fields = that.fields.values;
+
+for (var i=0; i fields.length; i++) {
+fields[i].clear();
+}
+};
+
 init();
 
 // methods that should be invoked by subclasses
@@ -720,9 +728,24 @@ IPA.details_facet = function(spec) {
 that.pre_execute_hook(command);
 }
 
+var old_pkey = that.old_pkey;
+that.old_pkey = that.pkey;
+
+if (that.pkey !== old_pkey) {
+that.clear();
+}
+
 command.execute();
 };
 
+that.clear = function() {
+var sections = that.sections.values;
+
+for (var i=0; i sections.length; i++) {
+sections[i].clear();
+}
+};
+
 that.add_sections(spec.sections);
 
 that.details_facet_create_content = that.create_content;
diff --git a/install/ui/host.js b/install/ui/host.js
index 4c0ce6ed0e461a38a565c1450cd483098b0c2dc7..ba28ebcf8ce3f176fcab06616733ae07ef36c976 100644
--- a/install/ui/host.js
+++ b/install/ui/host.js
@@ -567,6 +567,11 @@ IPA.host_keytab_widget = function(spec) {
 set_status(value ? 'present' : 'missing');
 };
 
+that.clear = function() {
+that.present_span.css('display', 'none');
+that.missing_span.css('display', 'none');
+};
+
 function set_status(status) {
 that.present_span.css('display', status == 'present' ? 'inline' : 'none');
 that.missing_span.css('display', status == 'missing' ? 'inline' : 'none');
@@ -720,6 +725,13 @@ IPA.host_password_widget = function(spec) {
 set_status(value ? 'present' : 'missing');
 };
 
+that.clear = function() {
+that.missing_span.css('display', 'none');
+that.present_span.css('display', 'none');
+var password_label = $('.button-label', that.set_password_button);
+password_label.text('');
+};
+
 function set_status(status) {
 
 that.status = status;
diff

Re: [Freeipa-devel] [PATCH] 028 Code cleanup of HBAC, Sudo rules

2011-10-27 Thread Petr Vobornik

I'll rebase this patch on recent patches (which will be soon pushed).

This effort is doing too many things, I'll split the patch into several.


On 10/25/2011 07:57 PM, Endi Sukma Dewata wrote:

On 10/25/2011 7:49 AM, Petr Vobornik wrote:

2. The set_facet() is added to widget. I don't think we want to make
widget dependent on facet. The facet so far is only used by
IPA.sudo.enable_widget. In IPA.sudo.options_section the facet object is
passed as a parameter in spec.



Are you saying that dependency on facet in ALL widgets is bad and only
in a few is good, or in any is bad?


In general less dependency is better. More dependency will limit its
usage. This is inline with the goal to make purely HTML widget.

But if a widget is meant to be used only with a facet then it's
certainly ok to make it depends on facet. However, the rest should not
become unnecessarily dependent on facet too.

I think it doesn't matter if all is dependant when one is dependant.

Anyway currently all association table widgets are dependant
(association.js:386).


Let's use this as an example. The association table explicitly checks if
the facet is dirty and asks the user whether to update/reset/cancel.
Suppose we want to use this inside a dialog, it will still check whether
the current facet is dirty instead of the dialog itself, which may not
be what we want.


Removed. Using instead: that.entity.get_facet() and 
that.entity.get_primary_key().


But still I think it would be better to be able to get container 
(facet/dialog) for a widget. As you wrote, that.entity.get_facet() may 
not always be what we want.





Btw similar topic could be: How to get current entity's pkey?'.
Dependancy on facet.get_primary_key() or
IPA.nav.get_state(that.entity.name+'-pkey') seems wrong too.


Yes. Ideally we should have a path (REST-style URL) instead of the
current parameter-based URL. We should get the pkey from that path.


From dependency point of view, widgets would be still dependant on the 
implementation of navigation (IMHO bad).


But it seems that using entity.get_primary key partially solves the problem.

Maybe it would be better if navigation would inject pkey to entity. 
Entity wouldn't be that much dependant on navigation implementation.



3. When updating sudo rule, in the original code the rule is
enabled/disabled after all the modifications are done. In the new code
it's reversed. I'm not sure the importance of the execution order for
this particular situation, but suppose it is, is there a way to maintain
the original order?


I was thinking about a command or widget priority. The 'mod' command
would have some default priority or it would get it from facet. The
commands would be sorted by priority before adding them to batch. This
way we can set exact order in facet update.


It's possible, but managing priority could be problematic if they are
spread out, because we have to know all existing priorities before we
can add a new one.


The priority could be part of update info. As there is a field_info we 
can create a command_info: { command, priority }.

 The original code uses an ordered list of commands.
Before adding commands to batch_command, array of command_info objects 
would joined with 'mod' command with default priority and then sorted by 
priority.


If the priority was set on each widget, priority management will be on 
facet level, which may be fine. There can be some corner case like 
dynamic change of priority.



The order which is implemented now is reflecting the order in HBAC
details facet - there were errors when 'mod' was executed before
clearing associations.


Right, so for certain things the order is important.


4. The IPA.header_widget is not really a widget, it's actually part of
layout. In the current implementation a widget always corresponds to an
attribute, so it will be loaded, saved, etc. Since there is no attribute
name matching the header name currently this is not a problem, but it
can pollute the namespace.


This is part of widget nesting topic. In this manner composite_widget
isn't a proper widget too (it can correspond to several or none
attribute).

Purely html rendering widgets can be separated from attribute widgets,
but it would be nice to have them because they can provide easier form
composing. Without them it would be required to override the create
method (in facet or composite_widget/section) which is sometimes better
but often it creates only code duplication with little added logic.


One other solution is to split widgets into non-visual fields and purely
HTML components. Then in the facet we use separate lists for the fields
and HTML components. The fields will have a reference to the
corresponding HTML components. There can be more HTML components than
fields. But this will require a significant restructuring.


Maybe we can use hybrid solution: html widgets would be simple object 
with some properties and create() method. They should not be called 
widgets. They would

Re: [Freeipa-devel] [PATCH] 301 Added password field in user adder dialog.

2011-10-27 Thread Petr Vobornik

On 10/27/2011 04:59 PM, Endi Sukma Dewata wrote:

On 10/27/2011 2:58 AM, Petr Vobornik wrote:

1) message in: 'field2.show_error('Passwords do not match');' should use
translated string.


Fixed. I'm considering to move the password fields into a reusable
section, but that will be a separate issue.


ACK

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 029 Page is cleared before it is visible

2011-11-01 Thread Petr Vobornik

On 10/31/2011 11:38 PM, Endi Sukma Dewata wrote:

On 10/27/2011 4:57 AM, Petr Vobornik wrote:

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



Some issues:

1. The new clear() method is called during refresh(), so the facet with
the old data is still shown for a brief moment before it's cleared.



The clear() should be called before show(). However, if the pkey/filter
is unchanged (check using needs_update()) we just need to show() the
facet, no need to clear() and refresh() again. The above logic needs to
be fixed.


Changed.



The we will need to override the needs_update() for search and
association facets because the default one always returns true.


Done


2. The following code in association.js and search.js will call clear()
if there's no old pkey/filter, is this intentional? No old pkey/filter
means the page is just loaded, so it probably doesn't need clearing.

if (!old_pkey || old_pkey[0] !== pkey[0]) {
that.clear();
}

if (!old_filter || old_filter[0] !== filter[0]) {
that.clear();
}
It seems unnecessary. But probably it was intentional (don't remember) - 
IDRC if there is a widget - maybe keytab or certificate status, which 
has some default state worth cleaning. Anyway in current implementation 
this logic is part of need_update and it is a must. IMHO we should avoid 
implementing special need_cleaning method. Cleaning at first display 
doesn't do any harm and it is one less method to maintain in couple classes.




3) Fixed bad implementation of clear method in radio_widget.

4) Changed direct/indirect radio names in association facets - radios 
form different facets were interfering.


5) Added ID generator, using in radio_widget, same reason as #4.

6) Added clearing of header in details facet and association facets - 
refreshes of member counts were confusing.


7) Removed setting that.pkey in create method in details, association 
facet (it broke need_update, didn't found purpose).


8) Maybe we should add a refresh button to search facet. It doesn't 
reflect concurrent usage. Refresh by 2 changes of filter doesn't seem nice.



--
Petr Vobornik
From d99d152ea71f89459b4cdb2b60690cc771e1b8fc Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Mon, 24 Oct 2011 14:53:29 +0200
Subject: [PATCH] Page is cleared before it is visible

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

Changes:
 * added clear method to widgets, section, search, details, association facets
 * clear and refresh method in facet are called only if key/filter was changed
 * added id generator for widgets
---
 install/ui/association.js |   21 ---
 install/ui/certificate.js |9 +
 install/ui/details.js |   21 ++--
 install/ui/entity.js  |   19 +--
 install/ui/host.js|   12 +++
 install/ui/search.js  |   18 --
 install/ui/service.js |5 +++
 install/ui/user.js|5 +++
 install/ui/widget.js  |   80 +++--
 9 files changed, 164 insertions(+), 26 deletions(-)

diff --git a/install/ui/association.js b/install/ui/association.js
index d3b66132d5043b0dfe60b8847896e9f27f676059..d3d6b124b431414ff04fad05b16dbb972b38c2b7 100644
--- a/install/ui/association.js
+++ b/install/ui/association.js
@@ -871,8 +871,6 @@ IPA.association_facet = function (spec) {
 
 that.facet_create_header(container);
 
-that.pkey = IPA.nav.get_state(that.entity.name+'-pkey');
-
 if (!that.read_only) {
 that.remove_button = IPA.action_button({
 name: 'remove',
@@ -908,12 +906,13 @@ IPA.association_facet = function (spec) {
 span.append(IPA.messages.association.show_results);
 span.append(' ');
 
-var direct_id = that.entity.name+'-'+that.attribute_member+'-'+that.other_entity+'-direct-radio';
+var name = that.entity.name+'-'+that.attribute_member+'-'+that.other_entity+'-type-radio';
+var direct_id = name + '-direct';
 
 that.direct_radio = $('input/', {
 id: direct_id,
 type: 'radio',
-name: 'type',
+name: name,
 value: 'direct',
 click: function() {
 that.association_type = $(this).val();
@@ -929,12 +928,12 @@ IPA.association_facet = function (spec) {
 
 span.append(' ');
 
-var indirect_id = that.entity.name+'-'+that.attribute_member+'-'+that.other_entity+'-indirect-radio';
+var indirect_id = name + '-indirect';
 
 that.indirect_radio = $('input/', {
 id: indirect_id,
 type: 'radio',
-name: 'type',
+name: name,
 value: 'indirect',
 click: function() {
 that.association_type = $(this).val();
@@ -1201,6 +1200,16 @@ IPA.association_facet = function (spec) {
 command.execute();
 };
 
+that.clear = function

Re: [Freeipa-devel] [PATCH] 029 Page is cleared before it is visible

2011-11-02 Thread Petr Vobornik

On 11/01/2011 11:31 PM, Endi Sukma Dewata wrote:

On 11/1/2011 7:37 AM, Petr Vobornik wrote:

1. The new clear() method is called during refresh(), so the facet with
the old data is still shown for a brief moment before it's cleared.



The clear() should be called before show(). However, if the pkey/filter
is unchanged (check using needs_update()) we just need to show() the
facet, no need to clear() and refresh() again. The above logic needs to
be fixed.


Changed.


The we will need to override the needs_update() for search and
association facets because the default one always returns true.


Done


On the second thought, this might not be sufficient to detect the
changes in the list page. Try changing an attribute in an entry, then go
back to the list page, the list page will not show the updated attribute
because the filter is not changed.

I think we should remove the needs_update() from the search facet so it
will always refresh, but we probably can keep it for association facet.
What do you think? Sorry about that.


Changed to refresh always. Clearing always or not clearing at all seems 
wrong. When the pkey doesn't change, only small portion of values 
can/will change, so it's probably not so wrong to show the previous 
list, but when the filter is changed the results will be probably much 
different and the clear is needed. What do you think? - Added 
needs_clear method which clears if the filter changes (basically renamed 
needs_update).



There are probably better solutions, but let's do this separately.


In future we can build a mechanism for subscribing to events from 
different facets and doing appropriate actions.


Something like:

var refresh_search_on_save = function(spec) {
var that = {};

that.register = function() {

that.entity = IPA.get_entity(spec.entity);
that.details_facet = that.entity.get_facet(spec.facet || 
'details');
that.search_facet = that.entity.get_facet(spec.search_facet || 
'search');


that.details_facet.on_save.attach(that.on_save, that);
};

that.on_save = function() {
that.search_facet.set_needs_refresh(true);
};

return that;
};

So the facets won't be dependent on each other.



5) Added ID generator, using in radio_widget, same reason as #4.


The get_id() method (might be better called get_next_id() or
generate_id()) doesn't really need to take a widget parameter. The
id_count should be unique enough. If you want, it can take an optional
prefix so the ID will be like 'prefix-id'. It will make it more
usable for other things not just widgets.


changed to get_next_id(prefix).





9. The facet header's clear() calls load() with empty data. The load()
will display the facet group label using facet's pkey. Since this is
called before refresh(), sometimes you'll see 'undefined' or the old
pkey. I think the code in entity.js:351-354 should check if the data is
empty it should clear the label.


Fixed


10. Instead of emptying button label in host.js:731-732, it's probably
better to reset it to its initial value:

var password_label = $('.button-label', that.set_password_button);
password_label.text(IPA.messages.objects.host.password_set_button);


Seems more proper to clean the label. If the label is set to 
...host.password_set_button it will always shows before load set OTP 
after load it can change to reset OTP which is wrong behavior.


--
Petr Vobornik
From b239957ae77de87bab163f3f43ca337d7f7bee33 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Mon, 24 Oct 2011 14:53:29 +0200
Subject: [PATCH] Page is cleared before it is visible

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

Changes:
 * added clear method to widgets, section, search, details, association facets
 * clear and refresh method in facet are called only if key/filter was changed
 * added id generator for widgets
---
 install/ui/association.js |   21 ---
 install/ui/certificate.js |9 +
 install/ui/details.js |   21 ++--
 install/ui/entity.js  |   28 +---
 install/ui/host.js|   12 +++
 install/ui/search.js  |   20 +--
 install/ui/service.js |5 +++
 install/ui/user.js|5 +++
 install/ui/widget.js  |   80 +++--
 9 files changed, 173 insertions(+), 28 deletions(-)

diff --git a/install/ui/association.js b/install/ui/association.js
index d3b66132d5043b0dfe60b8847896e9f27f676059..d3d6b124b431414ff04fad05b16dbb972b38c2b7 100644
--- a/install/ui/association.js
+++ b/install/ui/association.js
@@ -871,8 +871,6 @@ IPA.association_facet = function (spec) {
 
 that.facet_create_header(container);
 
-that.pkey = IPA.nav.get_state(that.entity.name+'-pkey');
-
 if (!that.read_only) {
 that.remove_button = IPA.action_button({
 name: 'remove',
@@ -908,12 +906,13 @@ IPA.association_facet = function (spec) {
 span.append

Re: [Freeipa-devel] [PATCH] 029 Page is cleared before it is visible

2011-11-02 Thread Petr Vobornik

On 11/02/2011 04:41 PM, Endi Sukma Dewata wrote:

On 11/2/2011 8:33 AM, Petr Vobornik wrote:

Question about this one:


4) Changed direct/indirect radio names in association facets - radios
form different facets were interfering.


Did you notice any problem with the old radio name? The name was
supposed to be used locally by the facet itself so it should not
interfere with radios in other facets, whereas ID is global so it needs
to be unique.


In sudo and hbac rule enable radio button.

Radios with same name are interfering on whole page. If you click at 
one, others gets unset. This wasn't an issue before because we have 
always reloaded the data. Radios in single facet wasn't interfering 
because they had different names. Gathering data wasn't a problem, 
because jquery selector was constraint to widget's container.



Regardless, ACK and pushed to master.


--
Petr Vobornik

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


[Freeipa-devel] [PATCH] 030 Extending facet's mechanism of gathering changes

2011-11-03 Thread Petr Vobornik

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

I'm not sure if update_info and other new classes should be in details.js.
--
Petr Vobornik
From 0506538ec9da347f2d2b7bac103b9c06fc405c2f Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Wed, 2 Nov 2011 16:43:00 +0100
Subject: [PATCH] Extending facet's mechanism of gathering changes

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

Adding option to gathering changes for update from widgets, sections, details facet.

Changes are represented by update_info { fields [] ((field_info)), commands [] ((command_info))  } object.

* On calling get_update_info() method widget, section and facet returns update_info object which represents all changes in nested objects. Thus usually widgets are creating update_infos, their containers are merging them.
* This object can be then used in details facet update method. In order to use it command_mode = 'init' has to be set. Command mode was introduced to support backward compatibility.
* command_info consists of command and priority. Priority can be set to specify exact exectuting order of commands. It can be defined on facet level by setting widget's priority. When widgit is creating command_info it should pas its priority to it.
---
 install/ui/details.js |  347 
 install/ui/ipa.js |   10 +-
 install/ui/widget.js  |   15 ++-
 3 files changed, 310 insertions(+), 62 deletions(-)

diff --git a/install/ui/details.js b/install/ui/details.js
index 15056204f72ef2095862c2c35d24cd47fbc819b3..621d47ecdd95672d530b78c0eb707e4af96002d8 100644
--- a/install/ui/details.js
+++ b/install/ui/details.js
@@ -207,6 +207,20 @@ IPA.details_section = function(spec) {
 }
 };
 
+that.get_update_info = function() {
+
+var update_info = IPA.update_info_builder.new_update_info();
+
+var fields = that.fields.values;
+for(var i=0; i  fields.length; i++) {
+update_info = IPA.update_info_builder.merge(
+update_info,
+fields[i].get_update_info());
+}
+
+return update_info;
+};
+
 init();
 
 // methods that should be invoked by subclasses
@@ -280,6 +294,8 @@ IPA.details_facet = function(spec) {
 that.entity = spec.entity;
 that.pre_execute_hook = spec.pre_execute_hook;
 that.post_update_hook = spec.post_update_hook;
+that.update_command_name = spec.update_command_name || 'mod';
+that.command_mode = spec.command_mode || 'save'; // [save, info]
 
 that.label = spec.label || IPA.messages  IPA.messages.facets  IPA.messages.facets.details;
 that.facet_group = spec.facet_group || 'settings';
@@ -398,11 +414,7 @@ IPA.details_facet = function(spec) {
 if (that.update_button.hasClass('action-button-disabled')) return false;
 
 if (!that.validate()) {
-var dialog = IPA.message_dialog({
-title: IPA.messages.dialogs.validation_title,
-message: IPA.messages.dialogs.validation_message
-});
-dialog.open();
+that.show_validation_error();
 return false;
 }
 
@@ -598,6 +610,7 @@ IPA.details_facet = function(spec) {
 that.enable_update(false);
 };
 
+
 that.validate = function() {
 var valid = true;
 var sections = that.sections.values;
@@ -608,46 +621,40 @@ IPA.details_facet = function(spec) {
 return valid;
 };
 
-that.update = function(on_win, on_fail) {
 
-function on_success(data, text_status, xhr) {
-if (on_win)
-on_win(data, text_status, xhr);
-if (data.error)
-return;
+that.on_update_success = function(data, text_status, xhr) {
 
-if (that.post_update_hook) {
-that.post_update_hook(data, text_status);
-return;
-}
+if (data.error)
+return;
 
-var result = data.result.result;
-that.load(result);
+if (that.post_update_hook) {
+that.post_update_hook(data, text_status);
+return;
 }
 
-function on_error(xhr, text_status, error_thrown) {
-if (on_fail)
-on_fail(xhr, text_status, error_thrown);
-}
+var result = data.result.result;
+that.load(result);
+};
+
+that.on_update_error = function(xhr, text_status, error_thrown) {
+};
 
-var args = that.get_primary_key();
+that.add_fields_to_command = function(update_info, command) {
 
-var command = IPA.command({
-entity: that.entity.name,
-method: 'mod',
-args: args,
-options: {
-all: true,
-rights: true
-},
-on_success: on_success,
-on_error: on_error

[Freeipa-devel] [PATCH] 031 Field for DNS SOA class changed to combobox with options

2011-11-03 Thread Petr Vobornik

https://fedorahosted.org/freeipa/ticket/602
--
Petr Vobornik
From 47022d96920b16a81ee54d53725de2e00d8c5c91 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Thu, 3 Nov 2011 15:14:15 +0100
Subject: [PATCH] Field for DNS SOA class changed to combobox with options

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

SOA class is an enumerated field. Changing input field to combobox with options allows inserting only valid value.
---
 install/ui/dns.js |   24 +++-
 1 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/install/ui/dns.js b/install/ui/dns.js
index 4dbf3e0d26699330b18285306ae7f6ee2c377324..769eee6020cd0be217ba64deada312b9a99ab2c9 100644
--- a/install/ui/dns.js
+++ b/install/ui/dns.js
@@ -62,7 +62,13 @@ IPA.entity_factories.dnszone = function() {
 'idnssoaexpire',
 'idnssoaminimum',
 'dnsttl',
-'dnsclass',
+{
+factory: IPA.combobox_widget,
+name: 'dnsclass',
+options: [
+'IN', 'CS', 'CH', 'HS'
+]
+},
 {
 factory: IPA.radio_widget,
 name: 'idnsallowdynupdate',
@@ -435,14 +441,14 @@ IPA.entity_factories.dnsrecord = function() {
 return IPA.entity_builder().
 entity('dnsrecord').
 containing_entity('dnszone').
-details_facet({
+details_facet({
 post_update_hook:function(data){
 var result = data.result.result;
  if (result.idnsname) {
 this.load(result);
 } else {
 this.reset();
-var dialog = IPA.dnsrecord_redirection_dialog();
+var dialog = IPA.dnsrecord_redirection_dialog();
 dialog.open(this.container);
 }
 },
@@ -603,11 +609,11 @@ IPA.entity_factories.dnsrecord = function() {
 };
 
 IPA.dnsrecord_redirection_dialog = function(spec) {
-spec = spec || {};
-spec.title = spec.title || IPA.messages.dialogs.redirection;  
-
-var that = IPA.dialog(spec);
-
+spec = spec || {};
+spec.title = spec.title || IPA.messages.dialogs.redirection;
+
+var that = IPA.dialog(spec);
+
 that.create = function() {
 $('p/', {
 'text': IPA.messages.objects.dnsrecord.deleted_no_data
@@ -616,7 +622,7 @@ IPA.dnsrecord_redirection_dialog = function(spec) {
 'text': IPA.messages.objects.dnsrecord.redirection_dnszone
 }).appendTo(that.container);
 };
-
+
 that.create_button({
 name: 'ok',
 label: IPA.messages.buttons.ok,
-- 
1.7.6.4

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

Re: [Freeipa-devel] [PATCH] 306 Moved facet code into facet.js.

2011-11-04 Thread Petr Vobornik

On 11/04/2011 04:35 AM, Endi Sukma Dewata wrote:

Facet-related code has been moved from entity.js into a new facet.js
because the file is getting too big.



ACK and pushed to master


--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 029 Page is cleared before it is visible

2011-11-04 Thread Petr Vobornik

On 11/03/2011 10:22 PM, Endi Sukma Dewata wrote:

On 11/2/2011 11:01 AM, Petr Vobornik wrote:

Regardless, ACK and pushed to master.


Found another problem, the krbtpolicy  config need to be forced to
update. See the attached patch.


ACK and pushed to master.

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 307 Added extensible UI framework.

2011-11-04 Thread Petr Vobornik

On 11/04/2011 04:37 AM, Endi Sukma Dewata wrote:

The entity definitions have been converted into classes. The entity
init() method will use the builder to construct the facets and dialogs.
The UI can be customized by creating a subclass of the original entity
in extension.js and then overriding the init() method.

Ticket #2043


There is a warning/error in browser when there is no extension.js 
present. This doesn't affect functionality, but I think we should try to 
eliminate this kinds of error. Same problem is for develop.js on 
production machines. This can be fixed separately.


ACK and pushed to master

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 308 Added current password field.

2011-11-07 Thread Petr Vobornik

On 11/04/2011 11:29 PM, Endi Sukma Dewata wrote:

The reset password dialog for user has been modified to provide
a field to specify the current password when changing the user's
own password.

Ticket #2065


ACK ipa-2-1, master

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 029 Page is cleared before it is visible

2011-11-07 Thread Petr Vobornik

On 11/05/2011 01:37 AM, Endi Sukma Dewata wrote:

On 11/4/2011 11:02 AM, Petr Vobornik wrote:
Found another problem, changing page in the association facet didn't
work because pkey is still the same. See the attached patch.



ACK and pushed to master

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 030 Extending facet's mechanism of gathering changes

2011-11-07 Thread Petr Vobornik

On 11/04/2011 06:22 PM, Endi Sukma Dewata wrote:

Rebased, ACK, and pushed to master. Some comments below.

On 11/4/2011 7:21 AM, Petr Vobornik wrote:

Perhaps the save_as_update_info() later can be merged with
get_update_info() too because both are essentially generating
update_info for dirty fields.



Yes, If we ensure that appropriate get_update_info() method will be in 
each custom widget/section, which isn't now.



Attached preview patch for #1515. Also attaching diff patch of reviewed
patch.


OK, I see how the enable widget creates the update info. How would you
handle the removal of users in HBAC rule when the usercategory is
changed to ALL?

This logic is part of rule_association_table_widget:rule.js:142. The 
radio buttons are triggering an event which disables/enables the asso. 
table. During get_update_info if the asso. table is disabled and it has 
entries in it, it creates a command which deletes the entries.

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 310 Updated sample data.

2011-11-11 Thread Petr Vobornik

On 11/09/2011 09:28 PM, Endi Sukma Dewata wrote:

New sample data files have been added for search facet paging. Unused
files have been removed. The names used in the files have been updated
for consistency.

Ticket #981


ACK


--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 311 Added paging on search facet.

2011-11-11 Thread Petr Vobornik

On 11/09/2011 09:28 PM, Endi Sukma Dewata wrote:

The search facet has been modified to support paging on most entities
using the --pkey-only option to get the primary keys and a batch command
to get the complete records.

Paging on DNS records is not supported because a record may appear as
multiple rows. The following entities do not have --pkey-only option:
Automount Key, Self-Service Permissions, Delegation.

The search and association facet have been refactored to reuse the
common code from the table facet base class.

Ticket #981



On facets where paging is not enabled is still shown pager (because 
page_length property is set). This can be misleading.


Otherwise it is OK.


--
Petr Vobornik

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


Re: [Freeipa-devel] Ticket #1976 - Tab color groups

2011-11-11 Thread Petr Vobornik

On 11/10/2011 10:23 PM, Endi Sukma Dewata wrote:

On 11/10/2011 2:00 PM, Kyle Baker wrote:

Attached a mockup which shows each tab in a color set. This offers a
quick visual reference as to the tab groupings.


The colored tab looks good. What are the color settings? Suppose we have
more than 3 tab groups what colors should we use?

+1



Also I have shown settings to the right as this is inconsistent
amongst the different sections of the tool. Setting should be
displayed to the right as users are more likely to focus on the
content.


It is consistent. Facet groups are always ordered in following way: 
'member', 'settings', 'memberof', 'managedby'. If some facet group 
doesn't have any facet, it isn't shown.  Changing the order can lead to 
the problem Endi pointed out:

It might not be the same for all entities. For groups the members are
probably more interesting, but for ACI permissions users are probably
more interested to see the settings rather than the privileges in which
it belongs to.


The problem might be changed into two separate questions:
 * How to order the facet groups?
 * Which facet show as default when displaying record? Right now it is 
the first facet of first facet group (source of the problem).


Should we have a 'summary' tab that displays the most important info,
then we provide links to go to the tab to edit the info. For example:

DNS Zone: example.com

| Summary | | Settings | | Records |


Settings [edit]
Name: example.com
Status: Active

Records [edit]
host1 A 192.168.1.1
host2 A 192.168.1.2


May be useful for some users.

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 314 Fixed entity definition in test cases.

2011-11-18 Thread Petr Vobornik

On 11/17/2011 08:25 PM, Endi Sukma Dewata wrote:

The test cases have been updated to use the new extensible mechanism
for defining and registering entities.

Ticket #2043



1) In the tests is a call of entity_init method which isn't defined 
anywhere. It leads to JS error and test fail.


--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 315 Added commands into metadata.

2011-11-18 Thread Petr Vobornik

On 11/17/2011 08:27 PM, Endi Sukma Dewata wrote:

The json_metadata command has been modified to return the command
metadata. The API.txt has been updated as well.

Needed by ticket #388



ACK from UI perspective. Ipalib changes seems fine to me, but I'm not 
100% sure.


Note: this patch depends on edewata-314 which isn't ACKed yet.
Note 2: I hope this is really needed. It adds 400KB of data to load at 
UI start.


--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] [WIP] 172+173+175 Create per-type DNS API

2011-12-02 Thread Petr Vobornik

On 12/02/2011 03:33 PM, Rob Crittenden wrote:

Martin Kosek wrote:

On Thu, 2011-12-01 at 17:18 -0500, Rob Crittenden wrote:

Martin Kosek wrote:

On Mon, 2011-11-28 at 17:35 +0100, Martin Kosek wrote:


3) The commands are in format dnsrecordRRTYPE-cmd, for example
dnsrecordmx-add. I think dnsrecord-mx-add may be more readable. If we
want to go this way, I would have to bend the server framework a
little
which parses an LDAP object from the command name (LDAP object name is
dnsrecordmx in this case). This is doable, although I am not sure if
this does not have some implications in WebUI side.

Martin


Rob, thank you for the review! What do you think about the 3 open
questions I posted above?


I haven't applied the patches to see what the output looks like yet so
can't really comment on the first two.

I think the extra dash would make the command line easier to grok, or at
least read, but it isn't a show stopper for me. I'd be interested in
feedback from the UI guys but they may have to start poking at it to
really know for sure how much of an issue it would be.


For UI it is better without the dash. With dash it breaks the 
entity-method naming, which is default behavior for creating commands to 
the server. But it is quite easy to implement it with the dash too. If 
it makes the CLI more usable we should add the dash - it may save users 
more time.



PTRRecord doc I think would read better as The hostname this reverse
record points to


Ok. Btw do you think it would be good to document this way all these new
DNSRecord part parameters? As I checked with Petr, these would be shown
in the UI - so the UI user would benefit from it. But it will require a
lot of writing and RFC study :-)


I was wondering that myself. The labels can be rather terse, I wasn't
sure how much more a _doc() would add. I was also wondering if we should
include some of the limits within the doc, esp the 0-64k ones since
those are smaller. It would make it somewhat inconsistent which is why I
didn't raise it.


The label for attributes is very useful, without it we would have to add 
the label into internal.py which would only complicate things. The doc 
can be the same as the label as it is now for many attributes. Anyway 
special doc is useful if something is not clear.



Martin

rob


--
Petr Vobornik

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


[Freeipa-devel] [PATCH] 051 Search facets show translated boolean values

2011-12-05 Thread Petr Vobornik
Created format method for getting translated messages for boolean values 
- IPA.boolean_column_format.


Used in hosts, sudo rules, hbac rules.

https://fedorahosted.org/freeipa/ticket/2027
--
Petr Vobornik
From df21f935ae6ce05ed0a4709aade99d9e94d2f810 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Mon, 5 Dec 2011 16:23:38 +0100
Subject: [PATCH] Search facets show translated boolean values

Created format method for getting translated messages for boolean values - IPA.boolean_column_format.

Used in hosts, sudo rules, hbac rules.

https://fedorahosted.org/freeipa/ticket/2027
---
 install/ui/hbac.js   |5 -
 install/ui/host.js   |3 ++-
 install/ui/sudo.js   |5 -
 install/ui/widget.js |   17 +
 4 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/install/ui/hbac.js b/install/ui/hbac.js
index cf13e459ee2128aefd0e539ac4e4aa86950c069b..82d92c5c53a19fffb8b8bf6a52bb340c32b85d85 100644
--- a/install/ui/hbac.js
+++ b/install/ui/hbac.js
@@ -39,7 +39,10 @@ IPA.hbac.rule_entity = function(spec) {
 search_all: true,
 columns: [
 'cn',
-'ipaenabledflag',
+{
+name: 'ipaenabledflag',
+format: IPA.boolean_column_format
+},
 'description'
 ]
 }).
diff --git a/install/ui/host.js b/install/ui/host.js
index 654b34de1ad99c3b80429b31c943d5d831940d6d..4da22df6fde52c6c9974edf1eefe263c62716df4 100644
--- a/install/ui/host.js
+++ b/install/ui/host.js
@@ -38,7 +38,8 @@ IPA.host.entity = function(spec) {
 'description',
 {
 name: 'has_keytab',
-label: IPA.messages.objects.host.enrolled
+label: IPA.messages.objects.host.enrolled,
+format: IPA.boolean_column_format
 }
 ]
 }).
diff --git a/install/ui/sudo.js b/install/ui/sudo.js
index 6e7aeca26792ccc19268b436ece2ddf12b4812b0..5163d152621bb09c66571f5efe15f4039c9f6cda 100644
--- a/install/ui/sudo.js
+++ b/install/ui/sudo.js
@@ -37,7 +37,10 @@ IPA.sudo.rule_entity = function(spec) {
 params.builder.search_facet({
 columns: [
 'cn',
-'ipaenabledflag',
+{
+name: 'ipaenabledflag',
+format: IPA.boolean_column_format
+},
 'description'
 ]
 }).
diff --git a/install/ui/widget.js b/install/ui/widget.js
index 5b50d8f16d339aaee499e1e00280eefe20a51545..b8fcbdeb0a17b96678bb5cbca9b88a4688d864bc 100644
--- a/install/ui/widget.js
+++ b/install/ui/widget.js
@@ -902,6 +902,23 @@ IPA.textarea_widget = function (spec) {
 return that;
 };
 
+IPA.boolean_column_format = function(value) {
+
+if (value instanceof Array) {
+value = value[0];
+}
+
+if (value === false || value === 'FALSE' || value === 'false') {
+return IPA.messages['false'];
+}
+
+if (value === true || value === 'TRUE' || value === 'true') {
+return IPA.messages['true'];
+}
+
+return '';
+};
+
 /*
   The entity name must be set in the spec either directly or via entity.name
 */
-- 
1.7.6.4

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

Re: [Freeipa-devel] [PATCH] 315 Added commands into metadata.

2011-12-06 Thread Petr Vobornik

On 12/06/2011 06:38 AM, Endi Sukma Dewata wrote:

On 11/18/2011 12:27 PM, Endi Sukma Dewata wrote:

Now the methods metadata seem to be a subset of commands metadata, so we
probably can change the UI to use commands metadata and not pull the
methods metadata.


I did this change in the updated patch. It seems to be working fine.


In the JSON API itself the parameters are specified as options, so the
order shouldn't matter to the UI. Is there a way to define the execute()
using unordered keywords? I'm trying to avoid changing the method
signature again in the future.


I replaced takes_args with takes_options which takes care the ordering
problem. I verified the old UI way of calling json_metadata still works.


Updated patch attached.


Web UI - ACK.
Server side - seems fine - I would give it an ACK, but I'm not sure if 
I'm the right person for it.



--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 316 Added support for radio buttons in table widget.

2011-12-06 Thread Petr Vobornik

On 12/06/2011 06:39 AM, Endi Sukma Dewata wrote:

On 11/21/2011 12:18 PM, Endi Sukma Dewata wrote:

The table widget has been modified to support single-valued attribute
using radio buttons needed by some facets in HBAC Test. The widget now
uses 'pagination' flag to determine whether to show the pagination
control. The test data has also been updated.

Ticket #388


Updated patch attached.



ACK
--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 317 Fixed entity metadata resolution.

2011-12-06 Thread Petr Vobornik

On 12/06/2011 06:39 AM, Endi Sukma Dewata wrote:

On 11/21/2011 12:23 PM, Endi Sukma Dewata wrote:

The current code assumes that an entity will always have a corresponding
LDAPObject on the server, so it looks for the metadata in a fixed
location. This assumption doesn't work for HBAC Test since it is a
Command, not an LDAPObject, so the metadata has to be obtained from a
different location. A new method get_default_metadata() has been added
to allow each entity to find the metadata from the correct location.

Ticket #388


Updated patch attached.


ACK

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 318 Refactored facet.load().

2011-12-06 Thread Petr Vobornik

On 12/06/2011 06:40 AM, Endi Sukma Dewata wrote:

On 11/21/2011 12:29 PM, Endi Sukma Dewata wrote:

The load() in IPA.facet has been modified to accept the complete
data returned by the server instead of just the result. This is
needed by HBAC Test to access other attributes returned in the
test result.

Ticket #388


Updated patch attached.


ACK


--
Petr Vobornik

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


  1   2   3   4   5   6   7   8   9   10   >