From 7179d6d4b8f9738f327607944cc544a045b27bfb Mon Sep 17 00:00:00 2001 From: spalger Date: Tue, 4 Apr 2017 18:02:55 -0700 Subject: [PATCH 1/3] [utils] move encodeQueryComponent into shared utils --- src/ui/public/utils/query_string.js | 24 +++--------------------- src/utils/encode_query_component.js | 19 +++++++++++++++++++ src/utils/index.js | 2 ++ src/utils/package_json.js | 25 ++++--------------------- 4 files changed, 28 insertions(+), 42 deletions(-) create mode 100644 src/utils/encode_query_component.js diff --git a/src/ui/public/utils/query_string.js b/src/ui/public/utils/query_string.js index c45be4b3f25f1..1080c0b9b2246 100644 --- a/src/ui/public/utils/query_string.js +++ b/src/ui/public/utils/query_string.js @@ -1,3 +1,5 @@ +import { encodeQueryComponent } from '../../../utils'; + const qs = {}; /***** @@ -12,26 +14,6 @@ function tryDecodeURIComponent(value) { catch (e) {} // eslint-disable-line no-empty } -/** - * This method is intended for encoding *key* or *value* parts of query component. We need a custom - * method because encodeURIComponent is too aggressive and encodes stuff that doesn't have to be - * encoded per http://tools.ietf.org/html/rfc3986: - * query = *( pchar / "/" / "?" ) - * pchar = unreserved / pct-encoded / sub-delims / ":" / "@" - * unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~" - * pct-encoded = "%" HEXDIG HEXDIG - * sub-delims = "!" / "$" / "&" / "'" / "(" / ")" - * / "*" / "+" / "," / ";" / "=" - */ -function encodeUriQuery(val, pctEncodeSpaces) { - return encodeURIComponent(val) - .replace(/%40/gi, '@') - .replace(/%3A/gi, ':') - .replace(/%24/g, '$') - .replace(/%2C/gi, ',') - .replace(/%20/g, (pctEncodeSpaces ? '%20' : '+')); -} - /** * Parses an escaped url query string into key-value pairs. * @returns {Object.} @@ -82,7 +64,7 @@ qs.encode = function (obj) { }; qs.param = function (key, val) { - return encodeUriQuery(key, true) + (val === true ? '' : '=' + encodeUriQuery(val, true)); + return encodeQueryComponent(key, true) + (val === true ? '' : '=' + encodeQueryComponent(val, true)); }; /** diff --git a/src/utils/encode_query_component.js b/src/utils/encode_query_component.js new file mode 100644 index 0000000000000..ba77e3d8d1fe8 --- /dev/null +++ b/src/utils/encode_query_component.js @@ -0,0 +1,19 @@ +/** + * This method is intended for encoding *key* or *value* parts of query component. We need a custom + * method because encodeURIComponent is too aggressive and encodes stuff that doesn't have to be + * encoded per http://tools.ietf.org/html/rfc3986: + * query = *( pchar / "/" / "?" ) + * pchar = unreserved / pct-encoded / sub-delims / ":" / "@" + * unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~" + * pct-encoded = "%" HEXDIG HEXDIG + * sub-delims = "!" / "$" / "&" / "'" / "(" / ")" + * / "*" / "+" / "," / ";" / "=" + */ +export function encodeQueryComponent(val, pctEncodeSpaces = false) { + return encodeURIComponent(val) + .replace(/%40/gi, '@') + .replace(/%3A/gi, ':') + .replace(/%24/g, '$') + .replace(/%2C/gi, ',') + .replace(/%20/g, (pctEncodeSpaces ? '%20' : '+')); +} diff --git a/src/utils/index.js b/src/utils/index.js index ba237b909b3be..a731f292a453b 100644 --- a/src/utils/index.js +++ b/src/utils/index.js @@ -5,6 +5,8 @@ export fromRoot from './from_root'; export pkg from './package_json'; export unset from './unset'; +export { encodeQueryComponent } from './encode_query_component'; + export { createConcatStream, createIntersperseStream, diff --git a/src/utils/package_json.js b/src/utils/package_json.js index 640db57157b8d..7bc29253fa447 100644 --- a/src/utils/package_json.js +++ b/src/utils/package_json.js @@ -1,22 +1,5 @@ -import { existsSync } from 'fs'; -import { join } from 'path'; +import { dirname } from 'path'; -let packageDir; -let packagePath; - -while (!packagePath || !existsSync(packagePath)) { - const prev = packageDir; - packageDir = prev ? join(prev, '..') : __dirname; - packagePath = join(packageDir, 'package.json'); - - if (prev === packageDir) { - // if going up a directory doesn't work, we - // are already at the root of the filesystem - throw new Error('unable to find package.json'); - } -} - - -module.exports = require(packagePath); -module.exports.__filename = packagePath; -module.exports.__dirname = packageDir; +module.exports = require('../../package.json'); +module.exports.__filename = require.resolve('../../package.json'); +module.exports.__dirname = dirname(module.exports.__filename); From f8318d1eb5e77aba2bbd7ec32abb40751bb48370 Mon Sep 17 00:00:00 2001 From: spalger Date: Tue, 4 Apr 2017 18:04:42 -0700 Subject: [PATCH 2/3] [uiNavLink] allow specifying subUrl pattern --- src/core_plugins/kibana/index.js | 1 + src/ui/public/chrome/api/nav.js | 15 ++++++++++----- src/ui/ui_nav_link.js | 7 +------ 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/core_plugins/kibana/index.js b/src/core_plugins/kibana/index.js index 92356b8fb67a5..8bab9a545f736 100644 --- a/src/core_plugins/kibana/index.js +++ b/src/core_plugins/kibana/index.js @@ -87,6 +87,7 @@ module.exports = function (kibana) { title: 'Dashboard', order: -1001, url: `${kbnBaseUrl}#/dashboard/list`, + subUrlBase: `${kbnBaseUrl}#/dashboard`, description: 'compose visualizations for much win', icon: 'plugins/kibana/assets/dashboard.svg', }, { diff --git a/src/ui/public/chrome/api/nav.js b/src/ui/public/chrome/api/nav.js index d5856e59b37d5..e84dda3344f42 100644 --- a/src/ui/public/chrome/api/nav.js +++ b/src/ui/public/chrome/api/nav.js @@ -1,5 +1,5 @@ import { parse, format } from 'url'; -import { startsWith, isString } from 'lodash'; +import { isString } from 'lodash'; export default function (chrome, internals) { chrome.getNavLinks = function () { @@ -110,7 +110,7 @@ export default function (chrome, internals) { const { appId, globalState: newGlobalState } = decodeKibanaUrl(url); for (const link of internals.nav) { - link.active = startsWith(url, link.url); + link.active = url.startsWith(link.subUrlBase); if (link.active) { setLastUrl(link, url); continue; @@ -124,11 +124,16 @@ export default function (chrome, internals) { } }; - internals.nav.forEach(link => { + function relativeToAbsolute(url) { // convert all link urls to absolute urls const a = document.createElement('a'); - a.setAttribute('href', link.url); - link.url = a.href; + a.setAttribute('href', url); + return a.href; + } + + internals.nav.forEach(link => { + link.url = relativeToAbsolute(link.url); + link.subUrlBase = relativeToAbsolute(link.subUrlBase); }); // simulate a possible change in url to initialize the diff --git a/src/ui/ui_nav_link.js b/src/ui/ui_nav_link.js index 209be61d6a5f8..409df41d2dc6c 100644 --- a/src/ui/ui_nav_link.js +++ b/src/ui/ui_nav_link.js @@ -1,11 +1,10 @@ -import { pick } from 'lodash'; - export default class UiNavLink { constructor(uiExports, spec) { this.id = spec.id; this.title = spec.title; this.order = spec.order || 0; this.url = `${uiExports.urlBasePath || ''}${spec.url}`; + this.subUrlBase = `${uiExports.urlBasePath || ''}${spec.subUrlBase || spec.url}`; this.description = spec.description; this.icon = spec.icon; this.linkToLastSubUrl = spec.linkToLastSubUrl === false ? false : true; @@ -13,8 +12,4 @@ export default class UiNavLink { this.disabled = spec.disabled || false; this.tooltip = spec.tooltip || ''; } - - toJSON() { - return pick(this, ['id', 'title', 'url', 'order', 'description', 'icon', 'linkToLastSubUrl', 'hidden', 'disabled', 'tooltip']); - } } From 2806dd26698075688f159876c3f43d4a29808b5c Mon Sep 17 00:00:00 2001 From: spalger Date: Tue, 4 Apr 2017 18:05:25 -0700 Subject: [PATCH 3/3] [dashboards] use dashboard urls that are more like before 5.3 --- src/core_plugins/kibana/index.js | 2 +- .../kibana/public/dashboard/dashboard.html | 2 +- .../kibana/public/dashboard/dashboard.js | 13 ++++--------- .../kibana/public/dashboard/dashboard_constants.js | 10 +++++++--- src/core_plugins/kibana/public/dashboard/index.js | 9 --------- .../public/dashboard/listing/dashboard_listing.js | 4 ++-- test/server_config.js | 2 +- 7 files changed, 16 insertions(+), 26 deletions(-) diff --git a/src/core_plugins/kibana/index.js b/src/core_plugins/kibana/index.js index 8bab9a545f736..eaed7600f353f 100644 --- a/src/core_plugins/kibana/index.js +++ b/src/core_plugins/kibana/index.js @@ -86,7 +86,7 @@ module.exports = function (kibana) { id: 'kibana:dashboard', title: 'Dashboard', order: -1001, - url: `${kbnBaseUrl}#/dashboard/list`, + url: `${kbnBaseUrl}#/dashboards`, subUrlBase: `${kbnBaseUrl}#/dashboard`, description: 'compose visualizations for much win', icon: 'plugins/kibana/assets/dashboard.svg', diff --git a/src/core_plugins/kibana/public/dashboard/dashboard.html b/src/core_plugins/kibana/public/dashboard/dashboard.html index e401b578a227b..379d23340f7bb 100644 --- a/src/core_plugins/kibana/public/dashboard/dashboard.html +++ b/src/core_plugins/kibana/public/dashboard/dashboard.html @@ -9,7 +9,7 @@ >
- Dashboard + Dashboard
{{ getDashTitle() }} diff --git a/src/core_plugins/kibana/public/dashboard/dashboard.js b/src/core_plugins/kibana/public/dashboard/dashboard.js index 230c713c22ab0..7f10ee96fe684 100644 --- a/src/core_plugins/kibana/public/dashboard/dashboard.js +++ b/src/core_plugins/kibana/public/dashboard/dashboard.js @@ -15,7 +15,7 @@ import dashboardTemplate from 'plugins/kibana/dashboard/dashboard.html'; import FilterBarQueryFilterProvider from 'ui/filter_bar/query_filter'; import DocTitleProvider from 'ui/doc_title'; import { getTopNavConfig } from './top_nav/get_top_nav_config'; -import { DashboardConstants } from './dashboard_constants'; +import { DashboardConstants, createDashboardEditUrl } from './dashboard_constants'; import { VisualizeConstants } from 'plugins/kibana/visualize/visualize_constants'; import UtilsBrushEventProvider from 'ui/utils/brush_event'; import FilterBarFilterBarClickHandlerProvider from 'ui/filter_bar/filter_bar_click_handler'; @@ -42,7 +42,7 @@ uiRoutes } } }) - .when(`${DashboardConstants.DASHBOARD_EDIT_PATH}/:id`, { + .when(createDashboardEditUrl(':id'), { template: dashboardTemplate, resolve: { dash: function (savedDashboards, Notifier, $route, $location, courier) { @@ -193,10 +193,7 @@ app.directive('dashboardApp', function (Notifier, courier, AppState, timefilter, function revertChangesAndExitEditMode() { dashboardState.resetState(); - const refreshUrl = dash.id ? - `${DashboardConstants.DASHBOARD_EDIT_PATH}/{{id}}` : DashboardConstants.CREATE_NEW_DASHBOARD_URL; - const refreshUrlOptions = dash.id ? { id: dash.id } : {}; - kbnUrl.change(refreshUrl, refreshUrlOptions); + kbnUrl.change(dash.id ? createDashboardEditUrl(dash.id) : DashboardConstants.CREATE_NEW_DASHBOARD_URL); // This is only necessary for new dashboards, which will default to Edit mode. updateViewMode(DashboardViewMode.VIEW); @@ -232,9 +229,7 @@ app.directive('dashboardApp', function (Notifier, courier, AppState, timefilter, if (id) { notify.info(`Saved Dashboard as "${dash.title}"`); if (dash.id !== $routeParams.id) { - kbnUrl.change( - `${DashboardConstants.DASHBOARD_EDIT_PATH}/{{id}}`, - { id: dash.id }); + kbnUrl.change(createDashboardEditUrl(dash.id)); } else { docTitle.change(dash.lastSavedTitle); updateViewMode(DashboardViewMode.VIEW); diff --git a/src/core_plugins/kibana/public/dashboard/dashboard_constants.js b/src/core_plugins/kibana/public/dashboard/dashboard_constants.js index 99da2b923d68b..e6190c7175c25 100644 --- a/src/core_plugins/kibana/public/dashboard/dashboard_constants.js +++ b/src/core_plugins/kibana/public/dashboard/dashboard_constants.js @@ -1,11 +1,15 @@ +import { encodeQueryComponent } from '../../../../utils'; export const DashboardConstants = { ADD_VISUALIZATION_TO_DASHBOARD_MODE_PARAM: 'addToDashboard', NEW_VISUALIZATION_ID_PARAM: 'addVisualization', - LANDING_PAGE_PATH: '/dashboard/list', + LANDING_PAGE_PATH: '/dashboards', // NOTE, the following two urls have to have LANDING_PAGE_PATH as their sub url for // the chrome nav links to work correctly (eg. so navigating away from an opened dashboard, and back to // the dashboard app, will reopen the dashboard, not show the landing page. - CREATE_NEW_DASHBOARD_URL: '/dashboard/list/create', - DASHBOARD_EDIT_PATH: '/dashboard/list/edit' + CREATE_NEW_DASHBOARD_URL: '/dashboard', }; + +export function createDashboardEditUrl(id) { + return `/dashboard/${encodeQueryComponent(id)}`; +} diff --git a/src/core_plugins/kibana/public/dashboard/index.js b/src/core_plugins/kibana/public/dashboard/index.js index 415800474a9a4..e3f03c0a7e80e 100644 --- a/src/core_plugins/kibana/public/dashboard/index.js +++ b/src/core_plugins/kibana/public/dashboard/index.js @@ -15,17 +15,8 @@ uiRoutes .defaults(/dashboard/, { requireDefaultIndex: true }) - .when('/dashboard', { - // For BWC, preserve unsaved dashboard links (i.e. links with state data and no ID) prior to v5.3 - redirectTo: DashboardConstants.CREATE_NEW_DASHBOARD_URL - }) .when(DashboardConstants.LANDING_PAGE_PATH, { template: dashboardListingTemplate, controller: DashboardListingController, controllerAs: 'listingController' - }) - .when('/dashboard/:id', { - // For BWC, preserve saved dashboard links prior to v5.3 - redirectTo: `${DashboardConstants.DASHBOARD_EDIT_PATH}/:id` }); - diff --git a/src/core_plugins/kibana/public/dashboard/listing/dashboard_listing.js b/src/core_plugins/kibana/public/dashboard/listing/dashboard_listing.js index 68145cbcdee57..85dddcf37d5b0 100644 --- a/src/core_plugins/kibana/public/dashboard/listing/dashboard_listing.js +++ b/src/core_plugins/kibana/public/dashboard/listing/dashboard_listing.js @@ -2,7 +2,7 @@ import SavedObjectRegistryProvider from 'ui/saved_objects/saved_object_registry' import 'ui/pager_control'; import 'ui/pager'; import _ from 'lodash'; -import { DashboardConstants } from '../dashboard_constants'; +import { DashboardConstants, createDashboardEditUrl } from '../dashboard_constants'; export function DashboardListingController($injector, $scope) { const $filter = $injector.get('$filter'); @@ -146,7 +146,7 @@ export function DashboardListingController($injector, $scope) { }; this.getUrlForItem = function getUrlForItem(item) { - return `#${DashboardConstants.DASHBOARD_EDIT_PATH}/${item.id}`; + return `#${createDashboardEditUrl(item.id)}`; }; this.getCreateDashboardHref = function getCreateDashboardHref() { diff --git a/test/server_config.js b/test/server_config.js index bb240491721e7..a2ba55cca1cd9 100644 --- a/test/server_config.js +++ b/test/server_config.js @@ -43,7 +43,7 @@ module.exports = { }, dashboard: { pathname: kibanaURL, - hash: '/dashboard/list', + hash: '/dashboards', }, settings: { pathname: kibanaURL,