[Freeipa-devel] [PATCH] 001 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. 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
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
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
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
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
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
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
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.
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.
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.
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.
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
[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.
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
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.
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.
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
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
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
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
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
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.
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
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
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
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.
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.
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
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
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.
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.
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
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.
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.
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
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
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
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.
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.
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.
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
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.
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.
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
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.
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
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.
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.
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.
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.
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.
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
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
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.
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.
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.
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
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.
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.
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
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
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
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
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
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
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
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.
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
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.
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.
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.
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.
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.
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.
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
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
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.
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
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
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
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
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
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.
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
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.
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.
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
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
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.
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.
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
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.
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.
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
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
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.
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.
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.
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().
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