From f6914dd89a22ec40f553dfb33ca409533273f796 Mon Sep 17 00:00:00 2001 From: Mike Murray Date: Fri, 2 Jun 2017 10:47:27 -0700 Subject: [PATCH 1/6] Implement missing router functions - Implement `Router.setQueryParam()` - Fix `Router.getRouteName()` --- imports/plugins/core/router/lib/router.js | 30 +++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/imports/plugins/core/router/lib/router.js b/imports/plugins/core/router/lib/router.js index e692573417b..bb3aae1bd32 100644 --- a/imports/plugins/core/router/lib/router.js +++ b/imports/plugins/core/router/lib/router.js @@ -70,7 +70,7 @@ class Router { static getRouteName() { const current = Router.current(); - return current.options && current.options.name || ""; + return current.route && current.route.name || ""; } static getParam(name) { @@ -87,6 +87,32 @@ class Router { return current.query && current.query[name] || undefined; } + static setQueryParams(newParams) { + const current = Router.current(); + const queryParams = Object.assign({}, current.query, newParams); + + for (const key in queryParams) { + if (queryParams[key] === null || queryParams[key] === undefined) { + delete queryParams[k]; + } + } + + Router.go(current.route.name, current.params, queryParams); + } + + static getRouteName(newParams) { + const current = Router.current(); + const queryParams = Object.assign({}, current.query, newParams); + + for (const key in queryParams) { + if (queryParams[key] === null || queryParams[key] === undefined) { + delete queryParams[k]; + } + } + + Router.go(current.route.name, current.params, queryParams); + } + static watchPathChange() { routerChangeDependency.depend(); } @@ -236,7 +262,7 @@ function hasRoutePermission(route) { /** - * getRouteName + * getRegistryRouteName * assemble route name to be standard * prefix/package name + registry name or route * @param {String} packageName [package name] From c15768367c423c2924c0cec2ddadbfe2fac88972 Mon Sep 17 00:00:00 2001 From: Mike Murray Date: Fri, 2 Jun 2017 11:06:20 -0700 Subject: [PATCH 2/6] Fix function name for `setQueryParams` --- imports/plugins/core/router/lib/router.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/imports/plugins/core/router/lib/router.js b/imports/plugins/core/router/lib/router.js index bb3aae1bd32..68f08cb4e63 100644 --- a/imports/plugins/core/router/lib/router.js +++ b/imports/plugins/core/router/lib/router.js @@ -100,7 +100,7 @@ class Router { Router.go(current.route.name, current.params, queryParams); } - static getRouteName(newParams) { + static setQueryParams(newParams) { const current = Router.current(); const queryParams = Object.assign({}, current.query, newParams); From 7e61447bf843521c16dd5662963b1a3811c8bc26 Mon Sep 17 00:00:00 2001 From: Mike Murray Date: Fri, 2 Jun 2017 19:11:25 -0700 Subject: [PATCH 3/6] Fixes and tests - Added jest config to package.json - Added some jest unit tests for Router - Added mocks for some dependencies - Fixed route matching in Router.isActiveClassName method --- .../core/router/__mocks__/meteor/blaze.js | 4 + .../meteor/gadicc:blaze-react-component.js | 1 + .../core/router/__mocks__/meteor/meteor.js | 4 + .../router/__mocks__/meteor/reactive-var.js | 0 .../router/__mocks__/meteor/templating.js | 0 .../core/router/__mocks__/meteor/tracker.js | 9 ++ .../core/router/lib/__tests__/router.spec.js | 106 ++++++++++++++++++ imports/plugins/core/router/lib/router.js | 29 +++-- package.json | 6 + 9 files changed, 149 insertions(+), 10 deletions(-) create mode 100644 imports/plugins/core/router/__mocks__/meteor/blaze.js create mode 100644 imports/plugins/core/router/__mocks__/meteor/gadicc:blaze-react-component.js create mode 100644 imports/plugins/core/router/__mocks__/meteor/meteor.js create mode 100644 imports/plugins/core/router/__mocks__/meteor/reactive-var.js create mode 100644 imports/plugins/core/router/__mocks__/meteor/templating.js create mode 100644 imports/plugins/core/router/__mocks__/meteor/tracker.js create mode 100644 imports/plugins/core/router/lib/__tests__/router.spec.js diff --git a/imports/plugins/core/router/__mocks__/meteor/blaze.js b/imports/plugins/core/router/__mocks__/meteor/blaze.js new file mode 100644 index 00000000000..f0e8711d4f5 --- /dev/null +++ b/imports/plugins/core/router/__mocks__/meteor/blaze.js @@ -0,0 +1,4 @@ +export const Blaze = { + remove() {}, + renderWithData() {} +}; diff --git a/imports/plugins/core/router/__mocks__/meteor/gadicc:blaze-react-component.js b/imports/plugins/core/router/__mocks__/meteor/gadicc:blaze-react-component.js new file mode 100644 index 00000000000..9d93bb09138 --- /dev/null +++ b/imports/plugins/core/router/__mocks__/meteor/gadicc:blaze-react-component.js @@ -0,0 +1 @@ +export default class BlazeComponent {} diff --git a/imports/plugins/core/router/__mocks__/meteor/meteor.js b/imports/plugins/core/router/__mocks__/meteor/meteor.js new file mode 100644 index 00000000000..c705c50d82e --- /dev/null +++ b/imports/plugins/core/router/__mocks__/meteor/meteor.js @@ -0,0 +1,4 @@ +export const Meteor = { + isClient: false, + isServer: true +}; diff --git a/imports/plugins/core/router/__mocks__/meteor/reactive-var.js b/imports/plugins/core/router/__mocks__/meteor/reactive-var.js new file mode 100644 index 00000000000..e69de29bb2d diff --git a/imports/plugins/core/router/__mocks__/meteor/templating.js b/imports/plugins/core/router/__mocks__/meteor/templating.js new file mode 100644 index 00000000000..e69de29bb2d diff --git a/imports/plugins/core/router/__mocks__/meteor/tracker.js b/imports/plugins/core/router/__mocks__/meteor/tracker.js new file mode 100644 index 00000000000..a61c26aee9c --- /dev/null +++ b/imports/plugins/core/router/__mocks__/meteor/tracker.js @@ -0,0 +1,9 @@ + +class Dependency { + depend() {} + changed() {} +} + +export const Tracker = { + Dependency +}; diff --git a/imports/plugins/core/router/lib/__tests__/router.spec.js b/imports/plugins/core/router/lib/__tests__/router.spec.js new file mode 100644 index 00000000000..2aadfc5ad56 --- /dev/null +++ b/imports/plugins/core/router/lib/__tests__/router.spec.js @@ -0,0 +1,106 @@ +jest.mock("/lib/collections", () => { + return { + Packages: {}, + Shops: {} + }; +}); + +import Router from "../router.js"; + + +/** + * Order Summary is a display only component + * It receives props and displays them accordingly + */ + +beforeEach(() => { + // Set some default routes + Router.setCurrentRoute({}); + Router.routes = [ + { + fullPath: "/test?filter=red", + query: { + filter: "red" + }, + route: { + name: "test", + path: "/test", + route: "/test" + } + }, + { + params: { + id: "a12345" + }, + query: { + filter: "green" + }, + route: { + fullPath: "/reaction/test/1234?filter=green", + group: { + prefix: "/reaction" + }, + name: "singleTestItem", + path: "/reaction/test/12345", + route: "/reaction/test/:id" + } + } + ]; +}); + +afterEach(() => { + jest.clearAllMocks(); +}); + + +test("Route name to equal test", () => { + // TODO: Update test when server router is implemented to use Router.go() + Router.setCurrentRoute(Router.routes[0]); + + expect(Router.getRouteName()).toBe("test"); +}); + +test("Route query param 'filter' to equal 'red'", () => { + // TODO: Update test when server router is implemented to use Router.go() + Router.setCurrentRoute(Router.routes[0]); + + const queryValue = Router.getQueryParam("filter"); + + expect(queryValue).toBe("red"); +}); + +test("Route /test/:id param 'id' to equal '12345'", () => { + // TODO: Update test when server router is implemented to use Router.go() + Router.setCurrentRoute(Router.routes[1]); + + const received = Router.getParam("id"); + + expect(received).toBe("a12345"); +}); + +test("Route is active with name 'singleTestItem'", () => { + // TODO: Update test when server router is implemented to use Router.go() + Router.setCurrentRoute(Router.routes[1]); + + const received = Router.isActiveClassName("singleTestItem"); + + expect(received).toBe("active"); +}); + +test("Route /test/:id is active with prefixed path '/reaction/test/12345'", () => { + // TODO: Update test when server router is implemented to use Router.go() + Router.setCurrentRoute(Router.routes[1]); + + const received = Router.isActiveClassName("/reaction/test/12345"); + + expect(received).toBe("active"); +}); + +test("Route /test is active with un-prefixed path '/test'", () => { + // TODO: Update test when server router is implemented to use Router.go() + Router.setCurrentRoute(Router.routes[0]); + + const received = Router.isActiveClassName("/test"); + + expect(received).toBe("active"); +}); diff --git a/imports/plugins/core/router/lib/router.js b/imports/plugins/core/router/lib/router.js index 68f08cb4e63..a646d9d6aa8 100644 --- a/imports/plugins/core/router/lib/router.js +++ b/imports/plugins/core/router/lib/router.js @@ -36,6 +36,7 @@ class Router { static Hooks = Hooks static routes = [] static _initialized = false; + static activeClassName = "active"; static set _routes(value) { Router.routes = value; @@ -225,18 +226,26 @@ Router.reload = () => { Router.isActiveClassName = (routeName) => { const current = Router.current(); const group = current.route.group; - const path = current.route.path; - let prefix; + let prefix = ""; - if (group && group.prefix) { - prefix = current.route.group.prefix; - } else { - prefix = ""; - } + if (current.route) { + const path = current.route.path; - if (typeof path === "string") { - const routeDef = path.replace(prefix + "/", ""); - return routeDef === routeName ? "active" : ""; + if (group && group.prefix) { + prefix = current.route.group.prefix; + } + + // Match route + if (prefix.length && routeName.startsWith(prefix) && path === routeName) { + // Route name is a path and starts with the prefix. (default '/reaction') + return Router.activeClassName; + } else if (routeName.startsWith("/") && path === routeName) { + // Route name isa path and starts with slash, but was not prefixed + return Router.activeClassName; + } else if (current.route.name === routeName) { + // Route name is the actual name of the route + return Router.activeClassName; + } } return ""; diff --git a/package.json b/package.json index 743c1d20252..89d4df54c59 100644 --- a/package.json +++ b/package.json @@ -139,5 +139,11 @@ "del-report": "rm -rf ~/allure-report && rm -rf allure-results", "test-local": "wdio tests/runner/local.conf.js", "test-browserstack": "wdio tests/runner/browserstack.conf.js; kill $(ps aux | grep '[b]rowserstack' | awk '{print $2}')" + }, + "jest": { + "moduleNameMapper": { + "^\/lib(.*)": "/lib/$1", + "^\/imports\/plugins(.*)": "/imports/plugins/$1" + } } } From 8e8eed6217988b475e6442550de0f32e5c7c36db Mon Sep 17 00:00:00 2001 From: hrath2015 Date: Tue, 6 Jun 2017 04:15:11 +0530 Subject: [PATCH 4/6] fix package.json to run tests (#2396) --- package.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 89d4df54c59..2a458de6551 100644 --- a/package.json +++ b/package.json @@ -143,7 +143,8 @@ "jest": { "moduleNameMapper": { "^\/lib(.*)": "/lib/$1", - "^\/imports\/plugins(.*)": "/imports/plugins/$1" + "^\/imports\/plugins(.*)": "/imports/plugins/$1", + "^meteor/(.*)": "/imports/plugins/core/router/__mocks__/meteor/$1" } } } From cd5ad96aa5b5bfc95cf2a509d9d82ef7e4d38b54 Mon Sep 17 00:00:00 2001 From: Mike Murray Date: Wed, 7 Jun 2017 20:09:39 -0700 Subject: [PATCH 5/6] Removed comment --- imports/plugins/core/router/lib/__tests__/router.spec.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/imports/plugins/core/router/lib/__tests__/router.spec.js b/imports/plugins/core/router/lib/__tests__/router.spec.js index 2aadfc5ad56..8b6b0f9bb7f 100644 --- a/imports/plugins/core/router/lib/__tests__/router.spec.js +++ b/imports/plugins/core/router/lib/__tests__/router.spec.js @@ -7,12 +7,6 @@ jest.mock("/lib/collections", () => { import Router from "../router.js"; - -/** - * Order Summary is a display only component - * It receives props and displays them accordingly - */ - beforeEach(() => { // Set some default routes Router.setCurrentRoute({}); From 5740adce2a5946d50133d4fc1c3e59542a4bcbab Mon Sep 17 00:00:00 2001 From: Mike Murray Date: Wed, 7 Jun 2017 20:10:58 -0700 Subject: [PATCH 6/6] Remove duplicated setQueryParams function Fixed variable --- imports/plugins/core/router/lib/router.js | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/imports/plugins/core/router/lib/router.js b/imports/plugins/core/router/lib/router.js index a646d9d6aa8..ea1497aade6 100644 --- a/imports/plugins/core/router/lib/router.js +++ b/imports/plugins/core/router/lib/router.js @@ -94,20 +94,7 @@ class Router { for (const key in queryParams) { if (queryParams[key] === null || queryParams[key] === undefined) { - delete queryParams[k]; - } - } - - Router.go(current.route.name, current.params, queryParams); - } - - static setQueryParams(newParams) { - const current = Router.current(); - const queryParams = Object.assign({}, current.query, newParams); - - for (const key in queryParams) { - if (queryParams[key] === null || queryParams[key] === undefined) { - delete queryParams[k]; + delete queryParams[key]; } }