Revised patch 118 attached.
I used:
* Forward first
* Forward only

and set 'default_value' to 'first'. So there would be always some value checked, which indicates what is actually used. There is a little issue with undo button if policy is not set '' because default_value !== ''.

I did this kinda in the hurry, so I hope I didn't missed anything crucial. Will be back on Tuesday.

On 04/05/2012 04:56 PM, Endi Sukma Dewata wrote:
On 4/4/2012 5:36 AM, Petr Vobornik wrote:
DNS forward policy fields were using mutually exclusive checkboxes. Such
behavior is unusual for users.

Checkboxes were changed to radios with new option 'none/default' to set
empty value ''.

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

Second patch removes mutex option from checkboxes. Not sure if needed.

Patch #118 might need some revision.

According to DNS docs the default forward policy is "first":

forward is only relevant in conjunction with a valid forwarders
statement. If set to 'only' the server will only forward queries,
if set to 'first' (default) it will send the queries to the forwarder
and if not answered will attempt to answer the query.

http://www.zytrax.com/books/dns/ch7/queries.html

So there are actually only 2 possible policies: first and only. Ideally
we should present 2 radio buttons:
* Forward first
* Forward only

However, in IPA the attribute is optional, so we have 3 ways to define
the policy:
* Default
* Forward first
* Forward only

This is OK and will match the CLI (I can ACK this), but people might be
asking 'What is the default policy?' and I'm not sure we document it
clearly.

If we're sure IPA's default forwarding policy is equivalent to DNS
default forwarding policy (i.e. first), the other alternative is to just
present 2 options like this:
* Forward first (default)
* Forward only

When loading the data, if there's no policy defined the UI will select
'first' by default. When saving the data, the UI can either store the
'first' value or normalize it into empty value.

Another thing, let's not use 'none' because it could be interpreted as
'do not forward'.

Also, it would be better to use a long description like "Forward first"
instead of just "first". In the DNS configuration it's meant to be read
like the longer description:

forward first;
forward only;

In the current UI we have "Forward policy" label which doesn't go well
with "first" or "only".

Patch #119 is ACKed, but feel free to keep the functionality if you
think it could be useful someday.



--
Petr Vobornik
From b652c7c55a8509c361c3b8b7ddaee10da2efab87 Mon Sep 17 00:00:00 2001
From: Petr Vobornik <pvobo...@redhat.com>
Date: Wed, 4 Apr 2012 10:49:16 +0200
Subject: [PATCH] DNS forward policy: checkboxes changed to radio buttons

DNS forward policy fields were using mutually exclusive checkboxes. Such behavior is unusual for users.

Checkboxes were changed to radios.

https://fedorahosted.org/freeipa/ticket/2599
---
 install/ui/dns.js                  |   30 ++++++++++++++++++++++++------
 install/ui/ipa.js                  |   21 +++++++++++++--------
 install/ui/test/data/ipa_init.json |    2 ++
 ipalib/plugins/internal.py         |    2 ++
 4 files changed, 41 insertions(+), 14 deletions(-)

diff --git a/install/ui/dns.js b/install/ui/dns.js
index 6a684b9ea207feee2425c8f14dd398918fcaca6b..0d7f03f20c0cf6a217a3fa3717bdd2f9bbefc1c9 100644
--- a/install/ui/dns.js
+++ b/install/ui/dns.js
@@ -52,10 +52,19 @@ IPA.dns.config_entity = function(spec) {
                             validators: [IPA.dnsforwarder_validator()]
                         },
                         {
-                            type: 'checkboxes',
+                            type: 'radio',
                             name: 'idnsforwardpolicy',
-                            mutex: true,
-                            options: IPA.create_options(['only', 'first'])
+                            default_value: 'first',
+                            options: [
+                                {
+                                    value: 'first',
+                                    label: IPA.messages.objects.dnsconfig.forward_first
+                                },
+                                {
+                                    value: 'only',
+                                    label: IPA.messages.objects.dnsconfig.forward_only
+                                }
+                            ]
                         },
                         'idnszonerefresh'
                     ]
@@ -170,10 +179,19 @@ IPA.dns.zone_entity = function(spec) {
                         validators: [IPA.dnsforwarder_validator()]
                     },
                     {
-                        type: 'checkboxes',
+                        type: 'radio',
                         name: 'idnsforwardpolicy',
-                        mutex: true,
-                        options: IPA.create_options(['only', 'first'])
+                        default_value: 'first',
+                        options: [
+                            {
+                                value: 'first',
+                                label: IPA.messages.objects.dnsconfig.forward_first
+                            },
+                            {
+                                value: 'only',
+                                label: IPA.messages.objects.dnsconfig.forward_only
+                            }
+                        ]
                     },
                     {
                         type: 'checkbox',
diff --git a/install/ui/ipa.js b/install/ui/ipa.js
index be87481018654c6803b9c610df3723566a4efac2..eeac030531302fffc0af79e70a835dca8120f674 100644
--- a/install/ui/ipa.js
+++ b/install/ui/ipa.js
@@ -1516,17 +1516,22 @@ IPA.limit_text = function(value, max_length) {
     return limited_text;
 };
 
-IPA.create_options = function(labels, values) {
-
-    if(!values) values = labels;
+IPA.create_options = function(values) {
 
     var options = [];
 
-    for (var i=0; i<labels.length; i++) {
-        options.push({
-            label: labels[i],
-            value: values[i]
-        });
+    for (var i=0; i<values.length; i++) {
+        var val = values[i];
+        var option = val;
+
+        if (typeof val === 'string') {
+            option = {
+                value: val,
+                label: val
+            };
+        }
+
+        options.push(option);
     }
 
     return options;
diff --git a/install/ui/test/data/ipa_init.json b/install/ui/test/data/ipa_init.json
index 6eed01e924abac09eaba843be7d2be8d5b75ce9c..441f7f00bfe250bc8ebeaffda7b609b622268ac9 100644
--- a/install/ui/test/data/ipa_init.json
+++ b/install/ui/test/data/ipa_init.json
@@ -201,6 +201,8 @@
                         },
                         "delegation": {},
                         "dnsconfig": {
+                            "forward_first": "Forward first",
+                            "forward_only": "Forward only",
                             "options": "Options"
                         },
                         "dnsrecord": {
diff --git a/ipalib/plugins/internal.py b/ipalib/plugins/internal.py
index 8ce3a00678fec4396cf5bbcdcd2b596bdb751820..08eefbce8cb5a9b852ebd326afb2da043cf95928 100644
--- a/ipalib/plugins/internal.py
+++ b/ipalib/plugins/internal.py
@@ -338,6 +338,8 @@ class i18n_messages(Command):
             "delegation": {
             },
             "dnsconfig": {
+                "forward_first": _("Forward first"),
+                "forward_only": _("Forward only"),
                 "options": _("Options"),
             },
             "dnsrecord": {
-- 
1.7.7.6

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

Reply via email to