On 07/17/2013 01:53 PM, Petr Vobornik wrote:
> On 07/16/2013 06:46 PM, Ana Krivokapic wrote:
>> Hello,
>>
>> This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3759.
>>
>
> Hello,
>
> Thanks for the patch, some comments:
>
> 1) idrange.js:183 I would avoid modifying widget html output in form methods.
> In this case you can simply add `layout: 'vertical'` to 'iparangetype' field
> definition.
>
> 2) idrange.js:187 Can be replaced by adding `enabled: false` to
> 'ipanttrusteddomainsid' field definition
>
> 3) I would rather see the switching logic encapsulated in a policy object than
> in a dialog. The main reason is to avoid using init() call in the factory.
> Most code other than method definitions in factory methods create mess in
> inheritance chain. Long term plan is to remove most of these calls. In this
> case, you can define public init method in the policy which will be
> automatically called after dialog instantiation.
>
> 4) IIUIC 'ipabaserid' have to be set together with 'ipanttrusteddomainsid' ->
> 'ipabaserid' should be made required when is_ad_trust is true.
>
> 5) As I read plugins/idrange.py:487-530, the logic for enabling/making
> required 'ipabaserid' and 'ipasecondarybaserid' is quite more complex than
> implemented.
>
> IIUIC 'ipasecondarybaserid' should be required and enabled only when
> 'ipabaserid' is set. Additionally, both should be required and enabled if
> adtrust_is_enabled (in UI: `IPA.trust_enabled`).

All fixed, updated patch attached.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From f7d64b5b773f935438a7479b9ecace3af50cc445 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic <akriv...@redhat.com>
Date: Tue, 16 Jul 2013 18:40:02 +0200
Subject: [PATCH] Expose ipaRangeType in Web UI

https://fedorahosted.org/freeipa/ticket/3759
---
 install/ui/src/freeipa/idrange.js  | 175 ++++++++++++++++++++++++++-----------
 install/ui/test/data/ipa_init.json |   5 +-
 ipalib/plugins/internal.py         |   3 +
 3 files changed, 131 insertions(+), 52 deletions(-)

diff --git a/install/ui/src/freeipa/idrange.js b/install/ui/src/freeipa/idrange.js
index 7e419e574c1e9ae08ad158d227e707055fc0a996..d92ba7359ad6af74f2e7df1ce72d93188f8660c5 100644
--- a/install/ui/src/freeipa/idrange.js
+++ b/install/ui/src/freeipa/idrange.js
@@ -85,85 +85,158 @@ return {
     adder_dialog: {
         fields: [
             {
-                name: 'cn',
-                widget: 'idrange.cn'
+                name: 'cn'
             },
             {
                 name: 'ipabaseid',
                 label: '@i18n:objects.idrange.ipabaseid',
-                tooltip: '@mo-param:idrange:ipabaseid:label',
-                widget: 'idrange.ipabaseid'
+                tooltip: '@mo-param:idrange:ipabaseid:label'
             },
             {
                 name: 'ipaidrangesize',
                 label: '@i18n:objects.idrange.ipaidrangesize',
-                tooltip: '@mo-param:idrange:ipaidrangesize:label',
-                widget: 'idrange.ipaidrangesize'
+                tooltip: '@mo-param:idrange:ipaidrangesize:label'
             },
             {
                 name: 'ipabaserid',
                 label: '@i18n:objects.idrange.ipabaserid',
-                tooltip: '@mo-param:idrange:ipabaserid:label',
-                widget: 'idrange.ipabaserid'
+                tooltip: '@mo-param:idrange:ipabaserid:label'
+            },
+            {
+                name: 'iparangetype',
+                $type: 'radio',
+                label: '@i18n:objects.idrange.type',
+                layout: 'vertical',
+                default_value: 'ipa-local',
+                options: [
+                    {
+                        value: 'ipa-local',
+                        label: '@i18n:objects.idrange.type_local'
+                    },
+                    {
+                        value: 'ipa-ad-trust',
+                        label: '@i18n:objects.idrange.type_ad'
+                    },
+                    {
+                        value: 'ipa-ad-trust-posix',
+                        label: '@i18n:objects.idrange.type_ad_posix'
+                    },
+                    {
+                        value: 'ipa-ad-winsync',
+                        label: '@i18n:objects.idrange.type_winsync'
+                    },
+                    {
+                        value: 'ipa-ipa-trust',
+                        label: '@i18n:objects.idrange.type_ipa'
+                    }
+                ]
             },
             {
                 name: 'ipasecondarybaserid',
                 label: '@i18n:objects.idrange.ipasecondarybaserid',
-                tooltip: '@mo-param:idrange:ipasecondarybaserid:label',
-                widget: 'type.ipasecondarybaserid'
+                tooltip: '@mo-param:idrange:ipasecondarybaserid:label'
             },
             {
                 name: 'ipanttrusteddomainsid',
                 label: '@i18n:objects.idrange.ipanttrusteddomainsid',
                 tooltip: '@mo-param:idrange:ipanttrusteddomainsid:label',
-                widget: 'type.ipanttrusteddomainsid'
-            }
-        ],
-        widgets: [
-            {
-                $type: 'details_table_section_nc',
-                name: 'idrange',
-                widgets: [
-                    'cn',
-                    'ipabaseid',
-                    'ipaidrangesize',
-                    'ipabaserid'
-                ]
-            },
-            {
-                $type: 'multiple_choice_section',
-                name: 'type',
-                label: '@i18n:objects.idrange.type',
-                choices: [
-                    {
-                        name: 'local',
-                        label: '@i18n:objects.idrange.type_local',
-                        fields: ['ipasecondarybaserid'],
-                        required: ['ipasecondarybaserid'],
-                        enabled: true
-                    },
-                    {
-                        name: 'ad',
-                        label: '@i18n:objects.idrange.type_ad',
-                        fields: ['ipanttrusteddomainsid'],
-                        required: ['ipanttrusteddomainsid']
-                    }
-                ],
-                widgets: [
-                    'ipasecondarybaserid',
-                    'ipanttrusteddomainsid'
-                ]
+                enabled: false
             }
         ],
         policies: [
-            {
-                $factory: IPA.multiple_choice_section_policy,
-                widget: 'type'
-            }
+                IPA.idrange_adder_policy
         ]
     }
 };};
 
+IPA.idrange_adder_policy = function(spec) {
+    /*
+    The logic for enabling/requiring ipabaserid, ipasecondarybaserid and
+    ipanttrusteddomainsid is as follows:
+        1) for AD ranges (range type is ipa-ad-trust or ipa-ad-trust-posix):
+           * ipabaserid and ipanttrusteddomainsid are requred
+           * ipasecondarybaserid is disabled
+        2) for local ranges
+           *  ipanttrusteddomainsid is disabled
+           A) if server has AD trust support:
+              * both ipabaserid and ipasecondarybaserid are required
+           B) if server does not have AD trust support:
+              * ipabaserid and ipasecondarybaserid may only be used together
+                (if one is set, other is required and vice versa)
+     */
+
+    function require(field) {
+        field.set_enabled(true);
+        field.set_required(true);
+    }
+
+    function disable(field) {
+        field.reset();
+        field.set_required(false);
+        field.set_enabled(false);
+    }
+
+    function enable(field) {
+        field.set_enabled(true);
+        field.set_required(false);
+    }
+
+    spec = spec || {};
+
+    var that = IPA.facet_policy(spec);
+
+    that.init = function() {
+        var type_f = that.container.fields.get_field('iparangetype');
+        var baserid_f = that.container.fields.get_field('ipabaserid');
+        var secondarybaserid_f = that.container.fields.get_field('ipasecondarybaserid');
+
+        if (IPA.trust_enabled) {
+            require(baserid_f);
+            require(secondarybaserid_f);
+        }
+
+        type_f.widget.value_changed.attach(that.on_input_change);
+        baserid_f.widget.value_changed.attach(that.on_input_change);
+        secondarybaserid_f.widget.value_changed.attach(that.on_input_change);
+    };
+
+    that.on_input_change = function() {
+        var type_f = that.container.fields.get_field('iparangetype');
+        var baserid_f = that.container.fields.get_field('ipabaserid');
+        var secondarybaserid_f = that.container.fields.get_field('ipasecondarybaserid');
+        var trusteddomainsid_f = that.container.fields.get_field('ipanttrusteddomainsid');
+
+        var type_v = type_f.save()[0];
+        var baserid_v = baserid_f.save()[0] || '';
+        var secondarybaserid_v = secondarybaserid_f.save()[0] || '';
+
+        var is_ad_range = (type_v === 'ipa-ad-trust' || type_v === 'ipa-ad-trust-posix');
+
+        if (is_ad_range) {
+            require(baserid_f);
+            require(trusteddomainsid_f);
+            disable(secondarybaserid_f);
+        } else {
+            disable(trusteddomainsid_f);
+
+            if (IPA.trust_enabled) {
+                require(baserid_f);
+                require(secondarybaserid_f);
+            } else {
+                if (baserid_v || secondarybaserid_v) {
+                    require(baserid_f);
+                    require(secondarybaserid_f);
+                } else {
+                    enable(baserid_f);
+                    enable(secondarybaserid_f);
+                }
+            }
+        }
+    };
+
+    return that;
+};
+
 exp.entity_spec = make_spec();
 exp.register = function() {
     var e = reg.entity;
diff --git a/install/ui/test/data/ipa_init.json b/install/ui/test/data/ipa_init.json
index b484cad1c87b6ca9ef71afed71788865eaaf6bcf..9cc7d23f7d25827bdccba0cb89524964720d1dab 100644
--- a/install/ui/test/data/ipa_init.json
+++ b/install/ui/test/data/ipa_init.json
@@ -381,7 +381,10 @@
                             "ipasecondarybaserid": "Secondary RID base",
                             "type": "Range type",
                             "type_ad": "Active Directory domain",
-                            "type_local": "Local domain"
+                            "type_ad_posix": "Active Directory domain with POSIX attributes",
+                            "type_local": "Local domain",
+                            "type_ipa": "IPA trust",
+                            "type_winsync": "Active Directory winsync"
                         },
                         "realmdomains": {
                             "identity": "Realm Domains",
diff --git a/ipalib/plugins/internal.py b/ipalib/plugins/internal.py
index 9bbc0c9f5782804e813e71bc2253c62b826a6806..b837412c78630dbb81a632850050d09e0ab0d029 100644
--- a/ipalib/plugins/internal.py
+++ b/ipalib/plugins/internal.py
@@ -516,7 +516,10 @@ class i18n_messages(Command):
                 "ipasecondarybaserid": _("Secondary RID base"),
                 "type": _("Range type"),
                 "type_ad": _("Active Directory domain"),
+                "type_ad_posix": _("Active Directory domain with POSIX attributes"),
                 "type_local": _("Local domain"),
+                "type_ipa": _("IPA trust"),
+                "type_winsync": _("Active Directory winsync"),
             },
             "realmdomains": {
                 "identity": _("Realm Domains"),
-- 
1.8.1.4

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

Reply via email to