Re: [Freeipa-devel] [PATCH] 0229-automount-delete-key

2011-06-01 Thread Adam Young

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

2011-06-01 Thread Endi Sukma Dewata

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

2011-06-01 Thread Adam Young


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

2011-05-31 Thread Endi Sukma Dewata

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

2011-05-31 Thread Adam Young

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

2011-05-31 Thread Adam Young
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

2011-05-31 Thread Adam Young

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

2011-05-31 Thread Adam Young

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) {