Re: [Freeipa-devel] [PATCH] 0229-automount-delete-key
On 06/01/2011 07:05 PM, Endi Sukma Dewata wrote: On 6/1/2011 10:09 AM, Adam Young wrote: 11. The option values for automount map adder dialog could be simplified to "direct" and "indirect". The values used are what is appended to the command's method. Had to leave them as is to keep that working. The option values (direct & indirect) could be translated internally into method names (add and add_indirect). It wouldn't matter for users, but it could improve clarity of Selenium tests. This is no big deal, it can be fixed another time. One more issue: 12. The map type radio buttons in the automount map adder dialog retain the previous selection. Try creating an indirect map, then click Add again, the indirect radio is selected but the mount point and parent map fields are not shown. It's ACKed. Issue #12 could be fixed before push. #12 was fixed and pushed. I also added sample data for automountmap_add_indirect. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0229-automount-delete-key
On 6/1/2011 10:09 AM, Adam Young wrote: 11. The option values for automount map adder dialog could be simplified to "direct" and "indirect". The values used are what is appended to the command's method. Had to leave them as is to keep that working. The option values (direct & indirect) could be translated internally into method names (add and add_indirect). It wouldn't matter for users, but it could improve clarity of Selenium tests. This is no big deal, it can be fixed another time. One more issue: 12. The map type radio buttons in the automount map adder dialog retain the previous selection. Try creating an indirect map, then click Add again, the indirect radio is selected but the mount point and parent map fields are not shown. It's ACKed. Issue #12 could be fixed before push. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0229-automount-delete-key
https://fedorahosted.org/freeipa/ticket/204 Comments for what was fixed are in the patch. Here's what I didn't change: 4. Clicking 'Back to List' when viewing a map brings you back to list of locations. Is this still intentional? Perhaps the label should be changed to 'Back to Locations' or simply hidden. Left it as is. if UXD objects, we can change in a follow on patch. 5. The conditional fields in IPA.dialog are a little bit limited because there is only one set of conditional fields which has to be enabled/disabled together. It might be better to replace the 'conditional' boolean paramter into 'field_group' then replace the enable/disable methods to accept a field group. This could be done later. Agreed. I'd like to merge this with the sections code used for aci 8. In dialog.js line 626 and search.js line 253, the hasOwnProperty() invocations are probably redundant because the key is obtained from the object itself, so that method will always return true. This falls under the rules from "Javascript the good parts" and is probably a good idea to leave in, even though in our code it is strictly unnecessary. 10. The 3rd level tab for automount key was removed. At this point does it makes sense to remove the 3rd level tabs completely? The 3rd tab will come back if/when we do autmountkey details. Leaving for now. 11. The option values for automount map adder dialog could be simplified to "direct" and "indirect". The values used are what is appended to the command's method. Had to leave them as is to keep that working. From f3a313b393a51b2a7e83e35882565ff5601f5ae0 Mon Sep 17 00:00:00 2001 From: Adam Young Date: Fri, 27 May 2011 11:32:17 -0400 Subject: [PATCH] automount delete key indirect automount maps code review changes for automount: Removed: fields for mount and parentmap in maps details since they are not present in show or mod Hid undo link for adder dialog set up click handler for checkboxes when row does not have primary key removed add override in automountmap_adder_dialog moved 'var input...' in automount.js line 158 to start of method. changed logic in if statmenet ,dialog.js line 628 it if (!first) as suggested --- install/ui/add.js |4 +- install/ui/automount.js | 81 +- install/ui/dialog.js| 51 - install/ui/search.js| 23 +++-- install/ui/webui.js |3 +- install/ui/widget.js| 15 + 6 files changed, 159 insertions(+), 18 deletions(-) diff --git a/install/ui/add.js b/install/ui/add.js index 73a423f00744394241638acceeb0dfa315af40cf..33df62abcaef6e5ec30e397b10d73ea5d8b478ff 100644 --- a/install/ui/add.js +++ b/install/ui/add.js @@ -32,7 +32,7 @@ IPA.add_dialog = function (spec) { that.name = spec.name; that.title = spec.title; that._entity_name = spec.entity_name; - +that.method = spec.method || 'add'; that.init = function() { that.add_button(IPA.messages.buttons.add, function() { @@ -102,7 +102,7 @@ IPA.add_dialog = function (spec) { var command = IPA.command({ entity: that.entity_name, -method: 'add', +method: that.method, on_success: on_success, on_error: on_error }); diff --git a/install/ui/automount.js b/install/ui/automount.js index f865fe73f315c224b53bbe2d74ff2f0109e4d6f2..f98b0b06cf498ac84ded0e3d48673438f1539bf2 100644 --- a/install/ui/automount.js +++ b/install/ui/automount.js @@ -63,7 +63,8 @@ IPA.entity_factories.automountmap = function() { nested_entity : 'automountkey', label : IPA.metadata.objects.automountkey.label, name: 'keys', -columns:['description'] +get_values: IPA.get_option_values, +columns:['automountkey','automountinformation'] }). details_facet({ sections:[ @@ -75,7 +76,27 @@ IPA.entity_factories.automountmap = function() { ] }). adder_dialog({ -fields:['automountmapname','description'] +factory: IPA.automountmap_adder_dialog, +fields:[{factory:IPA.method_radio_widget, + name: 'method', + undo: false, + label:'Map Type', + options:[{value:'add',label:'Direct'}, + {value:'add_indirect',label:'Indirect'}] +}, +'automountmapname','description', +{ +name:'key', +label:'Mount Point', +conditional:true, +undo: false +}, +{ +name:'parentmap', +label:'Parent Map', +conditional:true, +undo: false +
Re: [Freeipa-devel] [PATCH] 0229-automount-delete-key
On 5/31/2011 11:59 AM, Adam Young wrote: Some issues: 1. The labels for mount and parentmap fields in automount map are missing. 2. The mount and parentmap fields in automount map adder dialog shows undo button. This can be fixed by adding "undo: false" parameter. 3. The Delete button remains disabled after selecting some automount keys to be deleted. 4. Clicking 'Back to List' when viewing a map brings you back to list of locations. Is this still intentional? Perhaps the label should be changed to 'Back to Locations' or simply hidden. 5. The conditional fields in IPA.dialog are a little bit limited because there is only one set of conditional fields which has to be enabled/disabled together. It might be better to replace the 'conditional' boolean paramter into 'field_group' then replace the enable/disable methods to accept a field group. This could be done later. 6. The add() in IPA.automountmap_adder_dialog is probably unnecessary because it's only calling the superclass's add() method. 7. The following assignment in automount.js line 158: var input = $('input[name="'+that.name+'"]', that.container); could be moved to the beginning of the method to avoid reexecuting the same jQuery selector. 8. In dialog.js line 626 and search.js line 253, the hasOwnProperty() invocations are probably redundant because the key is obtained from the object itself, so that method will always return true. 9. The "if (first)" statement in dialog.js line 628 will only append a comma after the first key-value pair and nothing after that. This statement probably should have been "if (!first)" and moved to the beginning of the loop. 10. The 3rd level tab for automount key was removed. At this point does it makes sense to remove the 3rd level tabs completely? 11. The option values for automount map adder dialog could be simplified to "direct" and "indirect". -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0229-automount-delete-key
On 05/31/2011 10:00 AM, Adam Young wrote: On 05/31/2011 09:57 AM, Adam Young wrote: On 05/28/2011 08:54 PM, Adam Young wrote: ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel JSL lint cleanup ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel rebased on top of recent changes ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel From 8d142f02c0e42a58b49e335943e8f2375811062c Mon Sep 17 00:00:00 2001 From: Adam Young Date: Fri, 27 May 2011 11:32:17 -0400 Subject: [PATCH] automount delete key indirect automount maps --- install/ui/add.js |4 +- install/ui/automount.js | 77 +-- install/ui/dialog.js| 41 - install/ui/search.js| 23 -- install/ui/webui.js |3 +- install/ui/widget.js|2 +- 6 files changed, 138 insertions(+), 12 deletions(-) diff --git a/install/ui/add.js b/install/ui/add.js index 73a423f00744394241638acceeb0dfa315af40cf..33df62abcaef6e5ec30e397b10d73ea5d8b478ff 100644 --- a/install/ui/add.js +++ b/install/ui/add.js @@ -32,7 +32,7 @@ IPA.add_dialog = function (spec) { that.name = spec.name; that.title = spec.title; that._entity_name = spec.entity_name; - +that.method = spec.method || 'add'; that.init = function() { that.add_button(IPA.messages.buttons.add, function() { @@ -102,7 +102,7 @@ IPA.add_dialog = function (spec) { var command = IPA.command({ entity: that.entity_name, -method: 'add', +method: that.method, on_success: on_success, on_error: on_error }); diff --git a/install/ui/automount.js b/install/ui/automount.js index f865fe73f315c224b53bbe2d74ff2f0109e4d6f2..5b62bd912e85eb75a3570f1e2a06975d67180965 100644 --- a/install/ui/automount.js +++ b/install/ui/automount.js @@ -63,19 +63,30 @@ IPA.entity_factories.automountmap = function() { nested_entity : 'automountkey', label : IPA.metadata.objects.automountkey.label, name: 'keys', -columns:['description'] +get_values: IPA.get_option_values, +columns:['automountkey','automountinformation'] }). details_facet({ sections:[ { name:'identity', label: IPA.messages.details.identity, -fields:['automountmapname','description'] +fields:['automountmapname','description','mount','parentmap'] } ] }). adder_dialog({ -fields:['automountmapname','description'] +factory: IPA.automountmap_adder_dialog, +fields:[{factory:IPA.method_radio_widget, + name: 'method', + undo: false, + label:'Map Type', + options:[{value:'add',label:'Direct'}, + {value:'add_indirect',label:'Indirect'}] +}, +'automountmapname','description', +{name:'key',label:'Mount Point',conditional:true}, +{name:'parentmap', label:'Parent Map',conditional:true}] }). build(); }; @@ -99,3 +110,63 @@ IPA.entity_factories.automountkey = function() { }). build(); }; + + +IPA.automountmap_adder_dialog = function(spec){ +var that = IPA.add_dialog(spec); + +that.super_setup = that.setup; +that.setup = function(container) { +that.super_setup(container); +that.disable_conditional_fields(); +}; + +that.super_add = that.add; +that.add = function(record, on_success, on_error) { +that.super_add(record, on_success, on_error); +}; +return that; +}; + + +IPA.get_option_values = function(){ + +var values = []; +$('input[name="select"]:checked', this.table.tbody).each(function() { +var value = {}; +$('span',$(this).parent().parent()).each(function(){ +var name = this.attributes['name'].value; + +value[name] = $(this).text(); +}); +values.push (value); +}); +return values; +}; + +IPA.method_radio_widget = function(spec){ +var direct = true; + +var that = IPA.radio_widget(spec); + +that.setup = function(container) { + +$('input[name="'+that.name+'"]', that.container). +filter("[value="+ that.dialog.method+"]"). +attr('checked', true); + +var input = $('input[name="'+that.name+'"]', that.container); + +input.change(function() { +that.dialog.method = this.value; + +
Re: [Freeipa-devel] [PATCH] 0229-automount-delete-key
I thought I had posted my work for indirect maps as a different patch, but it appears not. This patch merged in delete key and indirect maps. On 05/31/2011 10:00 AM, Adam Young wrote: On 05/31/2011 09:57 AM, Adam Young wrote: On 05/28/2011 08:54 PM, Adam Young wrote: ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel JSL lint cleanup ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel rebased on top of recent changes ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0229-automount-delete-key
On 05/31/2011 09:57 AM, Adam Young wrote: On 05/28/2011 08:54 PM, Adam Young wrote: ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel JSL lint cleanup ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel rebased on top of recent changes From 8c08a8f3ac58bfa5ad22048ee288b54c0cc68eb2 Mon Sep 17 00:00:00 2001 From: Adam Young Date: Fri, 27 May 2011 21:37:05 -0400 Subject: [PATCH] automount delete key --- install/ui/automount.js | 31 ++- install/ui/dialog.js| 24 +++- install/ui/search.js| 23 --- install/ui/webui.js |3 +-- 4 files changed, 62 insertions(+), 19 deletions(-) diff --git a/install/ui/automount.js b/install/ui/automount.js index e5fab0d8f1311b2153b358675f57deff91858a55..5b62bd912e85eb75a3570f1e2a06975d67180965 100644 --- a/install/ui/automount.js +++ b/install/ui/automount.js @@ -63,7 +63,8 @@ IPA.entity_factories.automountmap = function() { nested_entity : 'automountkey', label : IPA.metadata.objects.automountkey.label, name: 'keys', -columns:['automountkey','automountinformation','description'] +get_values: IPA.get_option_values, +columns:['automountkey','automountinformation'] }). details_facet({ sections:[ @@ -81,7 +82,7 @@ IPA.entity_factories.automountmap = function() { undo: false, label:'Map Type', options:[{value:'add',label:'Direct'}, - {value:'add_indirect',label:'Indirect'}], + {value:'add_indirect',label:'Indirect'}] }, 'automountmapname','description', {name:'key',label:'Mount Point',conditional:true}, @@ -114,21 +115,35 @@ IPA.entity_factories.automountkey = function() { IPA.automountmap_adder_dialog = function(spec){ var that = IPA.add_dialog(spec); - - that.super_setup = that.setup; that.setup = function(container) { that.super_setup(container); that.disable_conditional_fields(); -} +}; that.super_add = that.add; that.add = function(record, on_success, on_error) { that.super_add(record, on_success, on_error); -} +}; return that; }; + +IPA.get_option_values = function(){ + +var values = []; +$('input[name="select"]:checked', this.table.tbody).each(function() { +var value = {}; +$('span',$(this).parent().parent()).each(function(){ +var name = this.attributes['name'].value; + +value[name] = $(this).text(); +}); +values.push (value); +}); +return values; +}; + IPA.method_radio_widget = function(spec){ var direct = true; @@ -151,9 +166,7 @@ IPA.method_radio_widget = function(spec){ that.dialog.disable_conditional_fields(); } }); - - -} +}; return that; }; diff --git a/install/ui/dialog.js b/install/ui/dialog.js index 092263df08bab6a08e3cd943359b58e7fdde76ed..a6a2692714bc358715abe809aa13ae703b7f90c3 100644 --- a/install/ui/dialog.js +++ b/install/ui/dialog.js @@ -51,7 +51,7 @@ IPA.dialog = function(spec) { that.conditional_fields[i] + ']',that.container).css('visibility','visible'); } -} +}; that.disable_conditional_fields = function(){ for (var i =0; i < that.conditional_fields.length; i+=1) { @@ -59,9 +59,7 @@ IPA.dialog = function(spec) { that.conditional_fields[i] + ']',that.container).css('visibility','hidden'); } -} - - +}; that.__defineGetter__("entity_name", function(){ return that._entity_name; @@ -620,8 +618,24 @@ IPA.deleter_dialog = function (spec) { ul.appendTo(div); for (var i=0; i',{ -'text': that.values[i] +'text': value }).appendTo(ul); } }; diff --git a/install/ui/search.js b/install/ui/search.js index 5786886ac8459e5b0e34bb881cc20707dcab19eb..ba27cc9ddf818c11003fe8f9a1c89c4c84d7a448 100644 --- a/install/ui/search.js +++ b/install/ui/search.js @@ -37,6 +37,12 @@ IPA.search_facet = function(spec) { that.search_all = spec.search_all || false; +function get_values (){ +return that.table.get_selected_values(); +} + +that.get_values = spec.get_values || get_values; + that.init = function() { that.facet_init(); that.managed_entity = IPA.get_entity(that.managed_entity_name); @@ -193,9 +199,10 @@ IPA.search_facet = function(spec) { that.remove_instances(that.managed_entity); }; + that.remove_instanc
Re: [Freeipa-devel] [PATCH] 0229-automount-delete-key
On 05/28/2011 08:54 PM, Adam Young wrote: ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel JSL lint cleanup From 7fba335cf391dab621102da43c3a1eae7cfdb74e Mon Sep 17 00:00:00 2001 From: Adam Young Date: Fri, 27 May 2011 21:37:05 -0400 Subject: [PATCH] automount delete key --- install/ui/automount.js | 31 ++- install/ui/dialog.js| 24 +++- install/ui/search.js| 23 --- install/ui/webui.js |3 +-- 4 files changed, 62 insertions(+), 19 deletions(-) diff --git a/install/ui/automount.js b/install/ui/automount.js index e5fab0d8f1311b2153b358675f57deff91858a55..5b62bd912e85eb75a3570f1e2a06975d67180965 100644 --- a/install/ui/automount.js +++ b/install/ui/automount.js @@ -63,7 +63,8 @@ IPA.entity_factories.automountmap = function() { nested_entity : 'automountkey', label : IPA.metadata.objects.automountkey.label, name: 'keys', -columns:['automountkey','automountinformation','description'] +get_values: IPA.get_option_values, +columns:['automountkey','automountinformation'] }). details_facet({ sections:[ @@ -81,7 +82,7 @@ IPA.entity_factories.automountmap = function() { undo: false, label:'Map Type', options:[{value:'add',label:'Direct'}, - {value:'add_indirect',label:'Indirect'}], + {value:'add_indirect',label:'Indirect'}] }, 'automountmapname','description', {name:'key',label:'Mount Point',conditional:true}, @@ -114,21 +115,35 @@ IPA.entity_factories.automountkey = function() { IPA.automountmap_adder_dialog = function(spec){ var that = IPA.add_dialog(spec); - - that.super_setup = that.setup; that.setup = function(container) { that.super_setup(container); that.disable_conditional_fields(); -} +}; that.super_add = that.add; that.add = function(record, on_success, on_error) { that.super_add(record, on_success, on_error); -} +}; return that; }; + +IPA.get_option_values = function(){ + +var values = []; +$('input[name="select"]:checked', this.table.tbody).each(function() { +var value = {}; +$('span',$(this).parent().parent()).each(function(){ +var name = this.attributes['name'].value; + +value[name] = $(this).text(); +}); +values.push (value); +}); +return values; +}; + IPA.method_radio_widget = function(spec){ var direct = true; @@ -151,9 +166,7 @@ IPA.method_radio_widget = function(spec){ that.dialog.disable_conditional_fields(); } }); - - -} +}; return that; }; diff --git a/install/ui/dialog.js b/install/ui/dialog.js index 092263df08bab6a08e3cd943359b58e7fdde76ed..a6a2692714bc358715abe809aa13ae703b7f90c3 100644 --- a/install/ui/dialog.js +++ b/install/ui/dialog.js @@ -51,7 +51,7 @@ IPA.dialog = function(spec) { that.conditional_fields[i] + ']',that.container).css('visibility','visible'); } -} +}; that.disable_conditional_fields = function(){ for (var i =0; i < that.conditional_fields.length; i+=1) { @@ -59,9 +59,7 @@ IPA.dialog = function(spec) { that.conditional_fields[i] + ']',that.container).css('visibility','hidden'); } -} - - +}; that.__defineGetter__("entity_name", function(){ return that._entity_name; @@ -620,8 +618,24 @@ IPA.deleter_dialog = function (spec) { ul.appendTo(div); for (var i=0; i',{ -'text': that.values[i] +'text': value }).appendTo(ul); } }; diff --git a/install/ui/search.js b/install/ui/search.js index 5786886ac8459e5b0e34bb881cc20707dcab19eb..ba27cc9ddf818c11003fe8f9a1c89c4c84d7a448 100644 --- a/install/ui/search.js +++ b/install/ui/search.js @@ -37,6 +37,12 @@ IPA.search_facet = function(spec) { that.search_all = spec.search_all || false; +function get_values (){ +return that.table.get_selected_values(); +} + +that.get_values = spec.get_values || get_values; + that.init = function() { that.facet_init(); that.managed_entity = IPA.get_entity(that.managed_entity_name); @@ -193,9 +199,10 @@ IPA.search_facet = function(spec) { that.remove_instances(that.managed_entity); }; + that.remove_instances = function(entity) { -var values = that.table.get_selected_values(); +var values = that.get_values(); var title; if (!values.length) { @@ -240,8 +247,16 @@ IPA.search_facet = function(spec) {