Hi,

Please review the attached patch. Thanks!

The navigation.js has been modified to make it more abstract, i.e.
unaware of entity facets. The nav_update_tabs() has been modified
such that it activates and updates the tabs based on the current
state stored in the URL.

The facets are now handled in entity.js. The ipa_entity_setup() has
been modified to update the facets based on the current state and
cached state.

The navigation.js also has been modified to be more class-like. The
nav_create() has been modified to store the tab configuration and
the tab container in internal variables nav_tabs_lists and
nav_container. The nav_update_tabs() now can be called without any
parameters.

Functions nav_push_state(), nav_get_state(), and nav_remove_state()
have been added to wrap BBQ API. This is to allow unit tests to
replace them with mockup functions to remove dependency on BBQ.

--
Endi S. Dewata
>From 842b882f8850e2702c6fc1aca55fc5658fe607c2 Mon Sep 17 00:00:00 2001
From: Endi S. Dewata <edew...@redhat.com>
Date: Thu, 30 Sep 2010 15:37:33 -0500
Subject: [PATCH] Refactoring navigation.js.

The navigation.js has been modified to make it more abstract, i.e.
unaware of entity facets. The nav_update_tabs() has been modified
such that it activates and updates the tabs based on the current
state stored in the URL.

The facets are now handled in entity.js. The ipa_entity_setup() has
been modified to update the facets based on the current state and
cached state.

The navigation.js also has been modified to be more class-like. The
nav_create() has been modified to store the tab configuration and
the tab container in internal variables nav_tabs_lists and
nav_container. The nav_update_tabs() now can be called without any
parameters.

Functions nav_push_state(), nav_get_state(), and nav_remove_state()
have been added to wrap BBQ API. This is to allow unit tests to
replace them with mockup functions to remove dependency on BBQ.
---
 install/static/entity.js                |   47 ++++++++++++++-
 install/static/navigation.js            |  102 +++++++++++--------------------
 install/static/test/navigation_tests.js |   98 +++++++++++++++++++++++++++---
 install/static/webui.js                 |   22 +++----
 4 files changed, 178 insertions(+), 91 deletions(-)

diff --git a/install/static/entity.js b/install/static/entity.js
index 7b82c06..39836fe 100644
--- a/install/static/entity.js
+++ b/install/static/entity.js
@@ -24,6 +24,9 @@ var ipa_entity_search_list = {};
 var ipa_entity_add_list = {};
 var ipa_entity_details_list = {};
 
+/* use this to track individual changes between two hashchange events */
+var window_hash_cache = {};
+
 function ipa_entity_set_search_definition(obj_name, data)
 {
     ipa_entity_search_list[obj_name] = data;
@@ -39,8 +42,49 @@ function ipa_entity_set_details_definition(obj_name, data)
     ipa_entity_details_list[obj_name] = data;
 }
 
-function ipa_entity_setup(jobj)
+function ipa_entity_setup(container)
 {
+    var id = container.attr('id');
+
+    var state = id + '-facet';
+    var facet = $.bbq.getState(state, true) || 'search';
+    var last_facet = window_hash_cache[state];
+
+    if (facet != last_facet) {
+        _ipa_entity_setup(container);
+        window_hash_cache[state] = facet;
+
+    } else if (facet == 'search') {
+        state = id + '-filter';
+        var filter = $.bbq.getState(state, true);
+        var last_filter = window_hash_cache[state];
+        if (filter == last_filter) return;
+
+        _ipa_entity_setup(container);
+        window_hash_cache[state] = filter;
+
+    } else if (facet == 'details') {
+        state = id + '-pkey';
+        var pkey = $.bbq.getState(state, true);
+        var last_pkey = window_hash_cache[state];
+        if (pkey == last_pkey) return;
+
+        _ipa_entity_setup(container);
+        window_hash_cache[state] = pkey;
+
+    } else if (facet == 'associate') {
+        state = id + '-enroll';
+        var enroll = $.bbq.getState(state, true);
+        var last_enroll = window_hash_cache[state];
+        if (enroll == last_enroll) return;
+
+        _ipa_entity_setup(container);
+        window_hash_cache[state] = enroll;
+    }
+}
+
+function _ipa_entity_setup(jobj) {
+
     var obj_name = jobj.attr('id');
 
     function reset_on_click() {
@@ -158,4 +202,3 @@ function ipa_entity_generate_views(obj_name, container, switch_view)
 
     container.append(ul);
 }
-
diff --git a/install/static/navigation.js b/install/static/navigation.js
index 3aa49fe..4135cb4 100644
--- a/install/static/navigation.js
+++ b/install/static/navigation.js
@@ -18,8 +18,23 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 */
 
-/* use this to track individual changes between two hashchange events */
-var window_hash_cache = {};
+var nav_tabs_lists;
+var nav_container;
+
+function nav_push_state(params)
+{
+    $.bbq.pushState(params);
+}
+
+function nav_get_state(key)
+{
+    return $.bbq.getState(key, true);
+}
+
+function nav_remove_state(key)
+{
+    $.bbq.removeState(key);
+}
 
 function nav_create(nls, container, tabclass)
 {
@@ -28,6 +43,9 @@ function nav_create(nls, container, tabclass)
     if (!tabclass)
         tabclass = 'tabs';
 
+    nav_tabs_lists = nls;
+    nav_container = container;
+
     nav_generate_tabs(nls, container, tabclass, 1);
 
     var tabs = $('.' + tabclass);
@@ -36,12 +54,12 @@ function nav_create(nls, container, tabclass)
             var state = {};
             var id = $(ui.panel).parent().attr('id');
             state[id] = ui.index;
-            $.bbq.pushState(state);
+            nav_push_state(state);
             return true;
         }
     });
 
-    nav_update_tabs(nls, container);
+    nav_update_tabs();
 }
 
 function nav_generate_tabs(nls, container, tabclass, depth)
@@ -88,76 +106,26 @@ function nav_create_tab_div(id)
     });
 }
 
-function nav_select_tabs(nls, container)
+function nav_update_tabs()
 {
-    var id = container.attr('id');
-    var selectedTab = $.bbq.getState(id, true) || 0;
-    if (selectedTab >= nls.length) selectedTab = 0;
-    container.tabs('select', selectedTab);
-
-    for (var i = 0; i < nls.length; ++i) {
-        var n = nls[i];
-
-        var div = $('#'+n.name);
-
-        if ( (!n.setup) && n.children) {
-            nav_select_tabs(n.children, div);
-        }
-    }
+    _nav_update_tabs(nav_tabs_lists, nav_container);
 }
 
-function nav_update_tabs(nls, container)
+function _nav_update_tabs(nls, container)
 {
-    nav_select_tabs(nls, container);
-
-    var index1 = container.tabs('option', 'selected');
-    if (index1 >= nls.length) return;
-
-    var tab1 = nls[index1];
-    if (!tab1.children) return;
-
-    var div1 = $('#' + tab1.name);
-    var index2 = div1.tabs('option', 'selected');
-    if (index2 >= tab1.children.length) return;
-
-    var tab2 = tab1.children[index2];
-    var obj_name = tab2.name;
-    var entity_setup = tab2.setup;
-    var div2 = $('#' + tab2.name);
-
-    var state = obj_name + '-facet';
-    var facet = $.bbq.getState(state, true) || 'search';
-    var last_facet = window_hash_cache[state];
-
-    if (facet != last_facet) {
-        entity_setup(div2);
-        window_hash_cache[state] = facet;
-
-    } else if (facet == 'search') {
-        state = obj_name + '-filter';
-        var filter = $.bbq.getState(state, true);
-        var last_filter = window_hash_cache[state];
-        if (filter == last_filter) return;
-
-        entity_setup(div2);
-        window_hash_cache[state] = filter;
+    var id = container.attr('id');
+    var index = nav_get_state(id);
+    if (!index || index >= nls.length) index = 0;
 
-    } else if (facet == 'details') {
-        state = obj_name + '-pkey';
-        var pkey = $.bbq.getState(state, true);
-        var last_pkey = window_hash_cache[state];
-        if (pkey == last_pkey) return;
+    container.tabs('select', index);
 
-        entity_setup(div2);
-        window_hash_cache[state] = pkey;
+    var tab = nls[index];
+    var container2 = $('#' + tab.name);
 
-    } else if (facet == 'associate') {
-        state = obj_name + '-enroll';
-        var enroll = $.bbq.getState(state, true);
-        var last_enroll = window_hash_cache[state];
-        if (enroll == last_enroll) return;
+    if (tab.children) {
+        _nav_update_tabs(tab.children, container2);
 
-        entity_setup(div2);
-        window_hash_cache[state] = enroll;
+    } else if (tab.setup) {
+        tab.setup(container2);
     }
 }
diff --git a/install/static/test/navigation_tests.js b/install/static/test/navigation_tests.js
index 4144e81..318ac66 100644
--- a/install/static/test/navigation_tests.js
+++ b/install/static/test/navigation_tests.js
@@ -27,7 +27,7 @@ test("Testing nav_create().", function() {
         [
             { name:'identity', label:'IDENTITY', children: [
                 {name:'user', label:'Users', setup:mock_setup_user},
-                {name:'group', label:'Users', setup:mock_setup_group},
+                {name:'group', label:'Users', setup:mock_setup_group}
             ]}];
     function  mock_setup_user (jobj){
         user_mock_called = true;
@@ -44,7 +44,7 @@ test("Testing nav_create().", function() {
     var navigation = $('<div id="navigation"/>').appendTo(document.body);
     var user_mock_called = false;
     var group_mock_called = false;
-    nav_create(mock_tabs_lists, navigation, 'tabs')
+    nav_create(mock_tabs_lists, navigation, 'tabs');
     ok(user_mock_called, "mock user setup was called");
     ok(!group_mock_called, "mock group setup was not called because the tab is inactive");
     same( navigation[0].children.length, 2, "Two Child tabs");
@@ -54,24 +54,104 @@ test("Testing nav_create().", function() {
     navigation.remove();
 });
 
-test("Testing  nav_select_tabs().", function() {
+test("Testing nav_update_tabs() with valid index.", function() {
 
+    var orig_push_state = nav_push_state;
+    var orig_get_state = nav_get_state;
+    var orig_remove_state = nav_remove_state;
+
+    var state = {};
+
+    nav_push_state = function(params) {
+        $.extend(state, params);
+    };
+    nav_get_state = function(key) {
+        return state[key];
+    };
+    nav_remove_state = function(key) {
+        delete state[key];
+    };
 
     var mock_tabs_lists =
         [
             { name:'identity', label:'IDENTITY', children: [
                 {name:'one', label:'One', setup: function (){}},
-                {name:'two', label:'Two', setup: function (){}},
+                {name:'two', label:'Two', setup: function (){}}
             ]}];
 
     var navigation = $('<div id="navigation"/>').appendTo(document.body);
 
-    nav_create(mock_tabs_lists, navigation, 'tabs')
+    nav_create(mock_tabs_lists, navigation, 'tabs');
+
+    nav_push_state({"identity":1});
+    nav_update_tabs();
+
+    same(
+        navigation.tabs('option', 'selected'), 0,
+        "Active tab at level 1"
+    );
 
-    $.bbq.pushState({"identity":2});
-    nav_select_tabs(mock_tabs_lists, navigation);
-    same( navigation[0].children[1].children[2].id, 'two', "Tab two");
-    $.bbq.removeState(["identity"]);
+    same(
+        $('#identity').tabs('option', 'selected'), 1,
+        "Active tab at level 2"
+    );
+
+    nav_remove_state("identity");
 
     navigation.remove();
+
+    nav_push_state = orig_push_state;
+    nav_get_state = orig_get_state;
+    nav_remove_state = orig_remove_state;
+});
+
+test("Testing nav_update_tabs() with out-of-range index.", function() {
+
+    var orig_push_state = nav_push_state;
+    var orig_get_state = nav_get_state;
+    var orig_remove_state = nav_remove_state;
+
+    var state = {};
+
+    nav_push_state = function(params) {
+        $.extend(state, params);
+    };
+    nav_get_state = function(key) {
+        return state[key];
+    };
+    nav_remove_state = function(key) {
+        delete state[key];
+    };
+
+    var mock_tabs_lists =
+        [
+            { name:'identity', label:'IDENTITY', children: [
+                {name:'one', label:'One', setup: function (){}},
+                {name:'two', label:'Two', setup: function (){}}
+            ]}];
+
+    var navigation = $('<div id="navigation"/>').appendTo(document.body);
+
+    nav_create(mock_tabs_lists, navigation, 'tabs');
+
+    nav_push_state({"identity":2});
+    nav_update_tabs();
+
+    same(
+        navigation.tabs('option', 'selected'), 0,
+        "Active tab at level 1"
+    );
+
+    same(
+        $('#identity').tabs('option', 'selected'), 0,
+        "Active tab at level 2"
+    );
+
+    nav_remove_state("identity");
+
+    navigation.remove();
+
+    nav_push_state = orig_push_state;
+    nav_get_state = orig_get_state;
+    nav_remove_state = orig_remove_state;
 });
diff --git a/install/static/webui.js b/install/static/webui.js
index ac28412..a354bff 100644
--- a/install/static/webui.js
+++ b/install/static/webui.js
@@ -30,7 +30,7 @@ var admin_tab_set = [
         {name:'host', label:'Hosts', setup: ipa_entity_setup},
         {name:'hostgroup', label:'Hostgroups', setup: ipa_entity_setup},
         {name:'netgroup', label:'Netgroups', setup: ipa_entity_setup},
-        {name:'service', label:'Services', setup: ipa_entity_setup},
+        {name:'service', label:'Services', setup: ipa_entity_setup}
     ]},
     {name:'policy', label:'POLICY', setup: unimplemented_tab},
     {name:'config', label:'CONFIG', children: [
@@ -38,16 +38,11 @@ var admin_tab_set = [
     ]}
 ];
 
-
-
-
-var self_serv_tabs_lists =
+var self_serv_tab_set =
     [
         { name:'identity', label:'IDENTITY', children: [
             {name:'user', label:'Users', setup:ipa_entity_setup}]}];
 
-var nav_tabs_lists;
-
 /* main (document onready event handler) */
 $(function() {
 
@@ -61,11 +56,15 @@ $(function() {
             $('#loggedinas').find('strong').text(whoami.krbprincipalname[0]);
             $('#loggedinas a').fragment(
                 {'user-facet':'details', 'user-pkey':whoami_pkey},2);
+
+            var navigation = $('#navigation');
+
             if (whoami.hasOwnProperty('memberof_rolegroup') &&
                 whoami.memberof_rolegroup.length > 0){
-                nav_tabs_lists = admin_tab_set;
+                nav_create(admin_tab_set, navigation, 'tabs');
+
             }else{
-                nav_tabs_lists = self_serv_tab_set;
+                nav_create(self_serv_tab_set, navigation, 'tabs');
 
                 var state = {'user-pkey':whoami_pkey ,
                              'user-facet': jQuery.bbq.getState('user-facet') ||
@@ -73,8 +72,6 @@ $(function() {
                 $.bbq.pushState(state);
             }
 
-            var navigation = $('#navigation');
-            nav_create(nav_tabs_lists, navigation, 'tabs');
 
             $('#login_header').html(ipa_messages.login.header);
         }else{
@@ -98,8 +95,7 @@ $(function() {
 /* main loop (hashchange event handler) */
 function window_hashchange(evt)
 {
-    var navigation = $('#navigation');
-    nav_update_tabs(nav_tabs_lists, navigation);
+    nav_update_tabs();
 }
 
 /* builder function for unimplemented tab content */
-- 
1.6.6.1

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

Reply via email to