URL: https://github.com/freeipa/freeipa/pull/4955 Author: serg-cymbaluk Title: #4955: [Backport][ipa-4-8] WebUI: Fix issue with opening links in new tab/window Action: opened
PR body: """ This PR was opened automatically because PR #4667 was pushed to master and backport to ipa-4-8 is required. """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/4955/head:pr4955 git checkout pr4955
From 7aa3e88ed3815b2fd0e9c0915f436a78a299c331 Mon Sep 17 00:00:00 2001 From: Serhii Tsymbaliuk <stsym...@redhat.com> Date: Wed, 6 May 2020 13:28:55 +0200 Subject: [PATCH 1/2] WebUI: Fix issue with opening links in new tab/window - fix table item links reference - fix global menu links reference - fix API browser side panel links - fix tab links reference Ticket: https://pagure.io/freeipa/issue/7137 Signed-off-by: Serhii Tsymbaliuk <stsym...@redhat.com> --- .../ui/src/freeipa/Application_controller.js | 34 +--------- install/ui/src/freeipa/association.js | 7 +- install/ui/src/freeipa/automount.js | 23 ++++--- install/ui/src/freeipa/facet.js | 26 ++++---- install/ui/src/freeipa/navigation.js | 42 ++++++++++-- install/ui/src/freeipa/navigation/routing.js | 64 +++++++++++++++++-- install/ui/src/freeipa/topology.js | 9 ++- install/ui/src/freeipa/widget.js | 20 +++++- .../src/freeipa/widgets/APIBrowserWidget.js | 11 +++- install/ui/src/freeipa/widgets/Menu.js | 43 ++++++++++++- 10 files changed, 195 insertions(+), 84 deletions(-) diff --git a/install/ui/src/freeipa/Application_controller.js b/install/ui/src/freeipa/Application_controller.js index a656b9061b..46aabc9c4d 100644 --- a/install/ui/src/freeipa/Application_controller.js +++ b/install/ui/src/freeipa/Application_controller.js @@ -541,37 +541,9 @@ define([ * Tries to find menu item with assigned facet and navigate to it. */ on_menu_click: function(menu_item) { - this._navigate_to_menu_item(menu_item); - }, - - _navigate_to_menu_item: function(menu_item) { - - if (menu_item.entity) { - // entity pages - routing.navigate([ - 'entity', - menu_item.entity, - menu_item.facet, - menu_item.pkeys, - menu_item.args]); - } else if (menu_item.facet) { - // concrete facets - routing.navigate(['generic', menu_item.facet, menu_item.args]); - } else { - // categories, select first posible child, it may be the last - var children = this.menu.query({parent: menu_item.name }); - if (children.total) { - var success = false; - for (var i=0; i<children.total;i++) { - success = this._navigate_to_menu_item(children[i]); - if (success) break; - } - } else { - return false; - } - } - - return true; + routing.navigate( + this.app_widget.menu_widget.get_item_path(menu_item) + ); }, /** diff --git a/install/ui/src/freeipa/association.js b/install/ui/src/freeipa/association.js index b083a79f9b..6e3d024d98 100644 --- a/install/ui/src/freeipa/association.js +++ b/install/ui/src/freeipa/association.js @@ -497,9 +497,10 @@ IPA.association_table_widget = function (spec) { column.entity = that.other_entity; if (column.link) { - column.link_handler = function(value) { - navigation.show_entity(that.other_entity.name, 'default', [value]); - return false; + column.get_entity_path = function(value) { + return navigation.get_entity_path( + that.other_entity.name, 'default', [value] + ); }; } } diff --git a/install/ui/src/freeipa/automount.js b/install/ui/src/freeipa/automount.js index bcfaf4b265..3b577be47a 100644 --- a/install/ui/src/freeipa/automount.js +++ b/install/ui/src/freeipa/automount.js @@ -268,19 +268,22 @@ IPA.automount_key_column = function(spec) { var info = record.automountinformation; if (info instanceof Array) info = info[0]; - $('<a/>', { - href: '#'+key, - text: key, - click: function() { + var pkeys = that.facet.get_pkeys(); + pkeys.push(key); - var pkeys = that.facet.get_pkeys(); - pkeys.push(key); + var args = { + info: info, + key: key + }; - var args = { - info: info, - key: key - }; + var entity_hash = navigation.get_entity_hash( + that.entity.name, 'details', pkeys, args + ); + $('<a/>', { + href: '#' + entity_hash, + text: key, + click: function() { navigation.show_entity(that.entity.name, 'details', pkeys, args); return false; } diff --git a/install/ui/src/freeipa/facet.js b/install/ui/src/freeipa/facet.js index ed09fd1181..d7702695d5 100644 --- a/install/ui/src/freeipa/facet.js +++ b/install/ui/src/freeipa/facet.js @@ -1639,7 +1639,9 @@ exp.FacetGroupsWidget = declare([], { $('<a/>', { text: tab.tab_label, 'class': 'tab-link', - href: "#" + navigation.create_hash(tab, {}), + href: "#" + navigation.create_hash(tab, { + pkeys: self.facet.get_pkeys() + }), name: tab.name }).appendTo(el); @@ -2383,16 +2385,15 @@ exp.table_facet = IPA.table_facet = function(spec, no_init) { }; /** + * Returns path to an entity details page * - * Method which will be called after clicking on pkey in table. + * It can be overridden by child classes. * - * It can be overridden by child classes for changing afterclick behavior. - * - * @param {String} value automatically filed by clicking - * @param {entity.entity} table entity - * @return {boolean} false + * @param {String} value - table column value + * @param {entity.entity} entity + * @return {Array} path to given entity */ - that.on_column_link_click = function(value, entity) { + that.get_column_entity_path = function(value, entity) { var pkeys = [value]; var args; @@ -2408,8 +2409,9 @@ exp.table_facet = IPA.table_facet = function(spec, no_init) { pkeys.push(value); } - navigation.show_entity(entity.name, that.details_facet_name, pkeys, args); - return false; + return navigation.get_entity_path( + entity.name, that.details_facet_name, pkeys, args + ); }; /** @@ -2453,8 +2455,8 @@ exp.table_facet = IPA.table_facet = function(spec, no_init) { } if (column.link && column.primary_key) { - column.link_handler = function(value) { - return that.on_column_link_click(value, entity); + column.get_entity_path = function(value) { + return that.get_column_entity_path(value, entity); }; } diff --git a/install/ui/src/freeipa/navigation.js b/install/ui/src/freeipa/navigation.js index 1797a5e921..20d202ba88 100644 --- a/install/ui/src/freeipa/navigation.js +++ b/install/ui/src/freeipa/navigation.js @@ -109,6 +109,20 @@ define([ } }, + /** + * Returns path to an entity details page base on routing params + */ + get_entity_path: function(entity_name, arg1, arg2, arg3) { + var params = {}; + this.set_params(params, arg1); + this.set_params(params, arg2); + this.set_params(params, arg3); + + return [ + 'entity', entity_name, params.facet, params.pkeys, params.args + ]; + }, + /** * Show entity facet * @@ -117,18 +131,32 @@ define([ * pkeys as Array * args as Object * @method show_entity - * @param String Enity name + * @param String entity_name * @param {Object|facet.facet|string|Function} arg1 * @param {Object|facet.facet|string|Function} arg2 * @param {Object|facet.facet|string|Function} arg3 */ show_entity: function(entity_name, arg1, arg2, arg3) { - var params = {}; - this.set_params(params, arg1); - this.set_params(params, arg2); - this.set_params(params, arg3); - return routing.navigate(['entity', entity_name, params.facet, - params.pkeys, params.args]); + var path = this.get_entity_path(entity_name, arg1, arg2, arg3); + return routing.navigate(path); + }, + + /** + * Return URL hash which refers to entity facet + * + * arg1,arg2,arg3 are: + * facet name as String + * pkeys as Array + * args as Object + * @method get_entity_hash + * @param String entity_name + * @param {Object|facet.facet|string|Function} arg1 + * @param {Object|facet.facet|string|Function} arg2 + * @param {Object|facet.facet|string|Function} arg3 + */ + get_entity_hash: function(entity_name, arg1, arg2, arg3) { + var path = this.get_entity_path(entity_name, arg1, arg2, arg3); + return path ? routing.get_hash(path): ''; }, /** diff --git a/install/ui/src/freeipa/navigation/routing.js b/install/ui/src/freeipa/navigation/routing.js index 89a323dd5f..8cef39b8c5 100644 --- a/install/ui/src/freeipa/navigation/routing.js +++ b/install/ui/src/freeipa/navigation/routing.js @@ -158,6 +158,19 @@ var routing = { return nav.navigate.apply(nav, path); }, + /** + * Return URL hash for a specific path + * + * @param {Array} path, e.g. ['entity', 'user', 'search'] + * @return {String} + */ + get_hash: function(path) { + path = path.slice(0); + var nav_name = path.shift(); + var nav = this.get_navigator(nav_name); + return nav.get_hash.apply(nav, path); + }, + /** * Navigate to specific facet with give options * @param {facets.Facet} facet @@ -428,12 +441,23 @@ routing.Navigator = declare([], { var facet = reg.facet.get(facet_name); if (!facet) { - routing.router._error('Unknown facet', 'navigation', { facet: facet_name}); + routing.router._error( + 'Unknown facet', 'navigation', { facet: facet_name} + ); return false; } if (!args) args = facet.get_state(); return routing.navigate_to_facet(facet, args); + }, + + get_hash: function(facet_name, args) { + var facet = reg.facet.get(facet_name); + + if (!facet) return ''; + if (!args) args = facet.get_state(); + + return routing.create_hash(facet, args); } }); @@ -449,20 +473,31 @@ routing.EntityNavigator = declare([routing.Navigator], { name: 'entity', - navigate: function(entity_name, facet_name, pkeys, args) { - + _get_facet: function(entity_name, facet_name) { var entity = reg.entity.get(entity_name); if (!entity) { - routing.router._error('Unknown entity', 'navigation', { entity: entity_name}); - return false; + routing.router._error( + 'Unknown entity', 'navigation', { entity: entity_name } + ); + return null; } var facet = entity.get_facet(facet_name); if (!facet) { - routing.router._error('Unknown facet', 'navigation', { facet: facet_name}); - return false; + routing.router._error( + 'Unknown facet', 'navigation', { facet: facet_name} + ); + return null; } + return facet; + }, + + navigate: function(entity_name, facet_name, pkeys, args) { + var facet = this._get_facet(entity_name, facet_name); + + if (!facet) return false; + // Use current state if none supplied if (!pkeys && !args) { args = facet.get_state(); @@ -473,6 +508,21 @@ routing.EntityNavigator = declare([routing.Navigator], { args.pkeys = facet.get_pkeys(pkeys); return routing.navigate_to_facet(facet, args); + }, + + get_hash: function(entity_name, facet_name, pkeys, args) { + var facet = this._get_facet(entity_name, facet_name); + + if (!facet) return ''; + + // Use current state if none supplied + if (!pkeys && !args) args = facet.get_state(); + args = args || {}; + + // Facets may be nested and require more pkeys than supplied. + args.pkeys = facet.get_pkeys(pkeys); + + return routing.create_hash(facet, args); } }); diff --git a/install/ui/src/freeipa/topology.js b/install/ui/src/freeipa/topology.js index 01a30d09c4..bf2c82e468 100644 --- a/install/ui/src/freeipa/topology.js +++ b/install/ui/src/freeipa/topology.js @@ -666,11 +666,10 @@ topology.serverroles_nested_search_facet = function(spec) { return { 'role_servrole': that.get_pkey() }; }; - that.on_column_link_click = function(value, entity) { - var pkeys = [value]; - - navigation.show_entity('server', that.details_facet_name, pkeys); - return false; + that.get_column_entity_path = function(value, entity) { + return navigation.get_entity_path( + 'server', that.details_facet_name, [value] + ); }; that.filter_records = function(records_map, pkey, record) { diff --git a/install/ui/src/freeipa/widget.js b/install/ui/src/freeipa/widget.js index 1948705938..ec2d10503f 100644 --- a/install/ui/src/freeipa/widget.js +++ b/install/ui/src/freeipa/widget.js @@ -39,6 +39,7 @@ define(['dojo/_base/array', './jquery', './metadata', './navigation', + './navigation/routing', './phases', './reg', './rpc', @@ -48,7 +49,8 @@ define(['dojo/_base/array', ], function(array, lang, construct, Evented, has, keys, on, string, topic, builder, config, datetime, entity_mod, IPA, $, - metadata, navigation, phases, reg, rpc, text, util, exp) { + metadata, navigation, routing, phases, reg, rpc, text, + util, exp) { /** * Widget module @@ -3529,7 +3531,7 @@ IPA.column = function (spec) { var c; if (that.link && !suppress_link) { c = $('<a/>', { - href: '#'+value, + href: '#' + routing.get_hash(that.get_entity_path(value)), click: function() { return that.link_handler(value); } @@ -3555,10 +3557,22 @@ IPA.column = function (spec) { // very simple implementation which doesn't handle navigation to // nested entities - navigation.show_entity(that.target_entity, that.target_facet, [value]); + var entity_path = that.get_entity_path(value); + routing.navigate(entity_path); return false; }; + /** + * Returns path to an entity details page + * + * @param {String} value - column value + * @return {Array} path to given entity + */ + that.get_entity_path = function(value) { + return navigation.get_entity_path( + that.target_entity, that.target_facet, [value] + ); + }; /*column initialization*/ if (that.entity && !that.label) { diff --git a/install/ui/src/freeipa/widgets/APIBrowserWidget.js b/install/ui/src/freeipa/widgets/APIBrowserWidget.js index ff1c8ee3f0..85a654e33e 100644 --- a/install/ui/src/freeipa/widgets/APIBrowserWidget.js +++ b/install/ui/src/freeipa/widgets/APIBrowserWidget.js @@ -74,7 +74,7 @@ widgets.APIBrowserWidget = declare([Stateful, Evented], { 'param': '@mo-param:' }, - _to_list: function(objects) { + _to_list: function(objects, type) { var names = []; for (name in objects) { if (objects.hasOwnProperty(name)) { @@ -86,6 +86,7 @@ widgets.APIBrowserWidget = declare([Stateful, Evented], { var o; for (var i=0,l=names.length; i<l; i++) { o = objects[names[i]]; + o.type = type; if (!o.name) o.name = names[i]; if (o.only_webui) continue; new_objects.push(o); @@ -95,7 +96,7 @@ widgets.APIBrowserWidget = declare([Stateful, Evented], { _get_commands: function() { var commands = metadata.get('@m:commands'); - commands = this._to_list(commands); + commands = this._to_list(commands, 'command'); return [{ name: "commands", label: "Commands", @@ -105,7 +106,7 @@ widgets.APIBrowserWidget = declare([Stateful, Evented], { _get_objects: function() { var objects = metadata.get('@m:objects'); - objects = this._to_list(objects); + objects = this._to_list(objects, 'object'); return [{ name: "commands", label: "Objects", @@ -364,6 +365,10 @@ widgets.APIBrowserWidget = declare([Stateful, Evented], { }.bind(this)); this.list_w = new ListViewWidget(); + this.list_w.create_item_link = function(item) { + return "#/p/apibrowser/type=" + item.type + "&name=" + item.name; + }; + this.object_detail_w = new browser_widgets.ObjectDetailWidget(); this.command_detail_w = new browser_widgets.CommandDetailWidget(); this.param_detail_w = new browser_widgets.ParamDetailWidget(); diff --git a/install/ui/src/freeipa/widgets/Menu.js b/install/ui/src/freeipa/widgets/Menu.js index 131be4104d..f9ed32c002 100644 --- a/install/ui/src/freeipa/widgets/Menu.js +++ b/install/ui/src/freeipa/widgets/Menu.js @@ -30,8 +30,10 @@ define(['dojo/_base/declare', 'dojo/Evented', 'dojo/on', '../jquery', - '../ipa'], function(declare, array, dom, construct, prop, dom_class, - dom_style, attr, query, Evented, on, $, IPA) { + '../ipa', + '../navigation/routing' + ], function(declare, array, dom, construct, prop, dom_class, + dom_style, attr, query, Evented, on, $, IPA, routing) { return declare([Evented], { /** @@ -193,6 +195,39 @@ define(['dojo/_base/declare', return this.level_class + '-' + level; }, + /** + * Return navigation path to a menu item + * + * @param {navigation.MenuItem} menu_item + * @return {Array} + */ + get_item_path: function(menu_item) { + if (menu_item.entity) { + // entity pages + return [ + 'entity', + menu_item.entity, + menu_item.facet, + menu_item.pkeys, + menu_item.args + ]; + } else if (menu_item.facet) { + // concrete facets + return ['generic', menu_item.facet, menu_item.args]; + } else { + // categories, select first possible child, it may be the last + var children = this._get_children(menu_item); + if (children.total) { + for (var i=0; i<children.total; i++) { + var child_path = this.get_item_path(children[i]); + if (child_path) return child_path; + } + } + } + + return null; + }, + /** * Updates content of li_node associated with menu_item base on * menu_item's state. @@ -222,8 +257,10 @@ define(['dojo/_base/declare', }); var a_node = query('a', li_node)[0]; + var item_path = this.get_item_path(menu_item); + var item_hash = item_path ? routing.get_hash(item_path) : ''; - prop.set(a_node, 'href', '#' + menu_item.name); + prop.set(a_node, 'href', '#' + item_hash); prop.set(a_node, 'textContent', menu_item.label); prop.set(a_node, 'title', menu_item.title || menu_item.label); }, From d572f1218ec227d40422dd68b594269f218c276a Mon Sep 17 00:00:00 2001 From: Serhii Tsymbaliuk <stsym...@redhat.com> Date: Mon, 11 May 2020 10:59:20 +0200 Subject: [PATCH 2/2] WebUI tests: Change navigation tests to find menu items using data-name instead of href Since menu pseudo-links was replaced with real one, navigation tests must be changed to not use href for searching items. Ticket: https://pagure.io/freeipa/issue/7137 Signed-off-by: Serhii Tsymbaliuk <stsym...@redhat.com> --- ipatests/test_webui/ui_driver.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ipatests/test_webui/ui_driver.py b/ipatests/test_webui/ui_driver.py index 70e8e7b256..2441411d0d 100644 --- a/ipatests/test_webui/ui_driver.py +++ b/ipatests/test_webui/ui_driver.py @@ -515,7 +515,7 @@ def navigate_by_menu(self, item, complete=True): parent = parts[0:-1] self.navigate_by_menu('/'.join(parent), complete) - s = ".navbar a[href='#%s']" % item + s = ".navbar li[data-name='%s'] a" % item link = self.find(s, By.CSS_SELECTOR, strict=True) assert link.is_displayed(), 'Navigation link is not displayed: %s' % item link.click()
_______________________________________________ FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/freeipa-devel@lists.fedorahosted.org
