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

2011-09-16 Thread Endi Sukma Dewata

On 9/15/2011 11:36 AM, Petr Vobornik wrote:

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.


Attached is the updated patch. I fixed the test case and did some 
cleanup. This is actually the problem I mentioned in the meeting (so 
patch 271 is actually fine).


It looks like currently the checkbox widget is only used with boolean 
attributes. And it will only work with boolean attributes anyway because 
right now the save() will always return a boolean value. But for now I'm 
trying to use the original loaded/default value unless it's 'TRUE' or 
'FALSE' which will be converted into boolean.


In the future we might want to use this with non-boolean attributes. So 
we need to be able to map a pair of non-boolean values into checked and 
unchecked state during load()/reset() and vice versa during save().


--
Endi S. Dewata
From a919724e7d22837a827e99bdfedfd5c6340326aa Mon Sep 17 00:00:00 2001
From: Endi S. Dewata edew...@redhat.com
Date: Wed, 14 Sep 2011 12:36:58 -0500
Subject: [PATCH] Fixed posix group checkbox.

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
---
 install/ui/group.js |   21 ++---
 install/ui/test/widget_tests.js |2 +-
 install/ui/widget.js|   27 +--
 3 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/install/ui/group.js b/install/ui/group.js
index 410a295d4ac98da161cee9455b910660ec608469..f49c00f0b3d94c2bedc30fceff2d0ee8f6662949 100644
--- a/install/ui/group.js
+++ b/install/ui/group.js
@@ -92,13 +92,28 @@ IPA.entity_factories.group =  function () {
 'cn',
 'description',
 {
-factory:IPA.checkbox_widget,
-name: 'posix',
+factory: IPA.group_nonposix_checkbox_widget,
+name: 'nonposix',
 label: IPA.messages.objects.group.posix,
 undo: false,
-checked: 'checked'
+checked: true
 },
 'gidnumber']
 }).
 build();
 };
+
+IPA.group_nonposix_checkbox_widget = function (spec) {
+
+spec = spec || {};
+
+var that = IPA.checkbox_widget(spec);
+
+that.save = function() {
+var value = that.checkbox_save()[0];
+// convert posix into non-posix
+return [!value];
+};
+
+return that;
+};
\ No newline at end of file
diff --git a/install/ui/test/widget_tests.js b/install/ui/test/widget_tests.js
index 9f0f6f0b59660a9c0648680ac94302ecf4d84aa5..caf28edcc8d577e38020c7dc3bae8aa7a3a7314b 100644
--- a/install/ui/test/widget_tests.js
+++ b/install/ui/test/widget_tests.js
@@ -241,7 +241,7 @@ test(Testing checkbox widget., function() {
 spec = {name:'title'};
 base_widget_test('test_value');
 
-mock_record = {'title':'something'};
+mock_record = { 'title': 'TRUE' };
 
 widget.load(mock_record);
 same(widget.save(),[true], Checkbox is set);
diff --git a/install/ui/widget.js b/install/ui/widget.js
index 58698486894ce9e72842ea1cf011a5fb75286421..5438c81575f2ae3b0eb6d86d4a90ec2749a37cf8 100644
--- a/install/ui/widget.js
+++ b/install/ui/widget.js
@@ -760,9 +760,11 @@ IPA.multivalued_text_widget = function(spec) {
 IPA.checkbox_widget = function (spec) {
 
 spec = spec || {};
+
 var that = IPA.widget(spec);
 
-that.checked = spec.checked || '';
+// default value
+that.checked = spec.checked || false;
 
 that.create = function(container) {
 
@@ -773,7 +775,7 @@ IPA.checkbox_widget = function (spec) {
 that.input = $('input/', {
 type: 'checkbox',
 name: that.name,
-checked : that.checked,
+checked: that.checked,
 

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

2011-09-15 Thread Petr Vobornik

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

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

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

Ticket #1799


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



--
Petr Vobornik

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


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

2011-09-14 Thread Endi Sukma Dewata

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

--
Endi S. Dewata
From 1dac389949b79ee83a58051c069138affa8c9894 Mon Sep 17 00:00:00 2001
From: Endi S. Dewata edew...@redhat.com
Date: Wed, 14 Sep 2011 12:36:58 -0500
Subject: [PATCH] Fixed posix group checkbox.

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
---
 install/ui/group.js  |   21 ++---
 install/ui/widget.js |   24 +++-
 2 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/install/ui/group.js b/install/ui/group.js
index 410a295d4ac98da161cee9455b910660ec608469..f8d42ea37fdbb3420008b332ca1a1717b3d36170 100644
--- a/install/ui/group.js
+++ b/install/ui/group.js
@@ -92,13 +92,28 @@ IPA.entity_factories.group =  function () {
 'cn',
 'description',
 {
-factory:IPA.checkbox_widget,
-name: 'posix',
+factory: IPA.nonposix_checkbox_widget,
+name: 'nonposix',
 label: IPA.messages.objects.group.posix,
 undo: false,
-checked: 'checked'
+checked: true
 },
 'gidnumber']
 }).
 build();
 };
+
+IPA.nonposix_checkbox_widget = function (spec) {
+
+spec = spec || {};
+
+var that = IPA.checkbox_widget(spec);
+
+that.save = function() {
+var value = that.checkbox_save()[0];
+// convert posix into non-posix
+return [!value];
+};
+
+return that;
+};
\ No newline at end of file
diff --git a/install/ui/widget.js b/install/ui/widget.js
index 58698486894ce9e72842ea1cf011a5fb75286421..d4a46bd37a9ccfac48469c312d81081105816b4f 100644
--- a/install/ui/widget.js
+++ b/install/ui/widget.js
@@ -760,9 +760,10 @@ IPA.multivalued_text_widget = function(spec) {
 IPA.checkbox_widget = function (spec) {
 
 spec = spec || {};
+
 var that = IPA.widget(spec);
 
-that.checked = spec.checked || '';
+that.checked = spec.checked;
 
 that.create = function(container) {
 
@@ -773,7 +774,7 @@ IPA.checkbox_widget = function (spec) {
 that.input = $('input/', {
 type: 'checkbox',
 name: that.name,
-checked : that.checked,
+checked: that.checked,
 title: that.tooltip,
 change: function() {
 that.set_dirty(that.test_dirty());
@@ -796,17 +797,22 @@ IPA.checkbox_widget = function (spec) {
 };
 
 that.update = function() {
-var value = that.values  that.values.length ? that.values[0] : false;
-if (value ===FALSE){
-value = false;
-}
-if (value ===TRUE){
-value = true;
+var checked = that.checked || false;
+if (that.values  that.values.length) {
+var value = that.values[0];
+if (value === FALSE) {
+checked = false;
+}
+if (value === TRUE) {
+checked = true;
+}
 }
 
-that.input.attr('checked', value);
+that.input.attr('checked', checked);
 };
 
+that.checkbox_save = that.save;
+
 return that;
 };
 
-- 
1.7.5.1

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