From 65cf56ad1208c42097854fb4e9f76de33cb6807e Mon Sep 17 00:00:00 2001 From: John Cowen Date: Tue, 11 Jun 2019 10:18:50 +0100 Subject: [PATCH] ui: Ensure Service Instance pages account for nodes (#5933) Include node name in the URL for service instances Integrate the node name slug into tests for service instance pages --- ui-v2/app/models/proxy.js | 1 + ui-v2/app/router.js | 2 +- ui-v2/app/routes/dc/services/instance.js | 4 ++-- ui-v2/app/services/repository/proxy.js | 2 +- ui-v2/app/services/repository/service.js | 7 +++++-- ui-v2/app/templates/dc/services/-instances.hbs | 2 +- .../acceptance/dc/services/instances/error.feature | 3 ++- .../acceptance/dc/services/instances/proxy.feature | 3 ++- .../acceptance/dc/services/instances/show.feature | 14 ++++++++++++-- .../dc/services/instances/sidecar-proxy.feature | 3 ++- .../dc/services/instances/with-proxy.feature | 3 ++- .../dc/services/instances/with-sidecar.feature | 3 ++- ui-v2/tests/pages/dc/services/instance.js | 2 +- 13 files changed, 34 insertions(+), 15 deletions(-) diff --git a/ui-v2/app/models/proxy.js b/ui-v2/app/models/proxy.js index 3e4fb5822..bb28d6ac0 100644 --- a/ui-v2/app/models/proxy.js +++ b/ui-v2/app/models/proxy.js @@ -8,5 +8,6 @@ export default Model.extend({ [SLUG_KEY]: attr('string'), ServiceName: attr('string'), ServiceID: attr('string'), + Node: attr('string'), ServiceProxy: attr(), }); diff --git a/ui-v2/app/router.js b/ui-v2/app/router.js index 50234f27d..d281ac887 100644 --- a/ui-v2/app/router.js +++ b/ui-v2/app/router.js @@ -19,7 +19,7 @@ export const routes = { _options: { path: '/:name' }, }, instance: { - _options: { path: '/:name/:id' }, + _options: { path: '/:name/:node/:id' }, }, }, // Nodes represent a consul node diff --git a/ui-v2/app/routes/dc/services/instance.js b/ui-v2/app/routes/dc/services/instance.js index 2fb9eb30f..4cd467b07 100644 --- a/ui-v2/app/routes/dc/services/instance.js +++ b/ui-v2/app/routes/dc/services/instance.js @@ -11,7 +11,7 @@ export default Route.extend({ const proxyRepo = get(this, 'proxyRepo'); const dc = this.modelFor('dc').dc.Name; return hash({ - item: repo.findInstanceBySlug(params.id, params.name, dc), + item: repo.findInstanceBySlug(params.id, params.node, params.name, dc), }).then(function(model) { // this will not be run in a blocking loop, but this is ok as // its highly unlikely that a service will suddenly change to being a @@ -20,7 +20,7 @@ export default Route.extend({ proxy: get(model.item, 'Kind') === 'connect-proxy' ? null - : proxyRepo.findInstanceBySlug(params.id, params.name, dc), + : proxyRepo.findInstanceBySlug(params.id, params.node, params.name, dc), ...model, }); }); diff --git a/ui-v2/app/services/repository/proxy.js b/ui-v2/app/services/repository/proxy.js index 6daeee8c5..2e74b39b0 100644 --- a/ui-v2/app/services/repository/proxy.js +++ b/ui-v2/app/services/repository/proxy.js @@ -19,7 +19,7 @@ export default RepositoryService.extend({ } return this.get('store').query(this.getModelName(), query); }, - findInstanceBySlug: function(id, slug, dc, configuration) { + findInstanceBySlug: function(id, node, slug, dc, configuration) { return this.findAllBySlug(slug, dc, configuration).then(function(items) { let res = {}; if (get(items, 'length') > 0) { diff --git a/ui-v2/app/services/repository/service.js b/ui-v2/app/services/repository/service.js index 472c6ae67..7b85acf54 100644 --- a/ui-v2/app/services/repository/service.js +++ b/ui-v2/app/services/repository/service.js @@ -32,10 +32,13 @@ export default RepositoryService.extend({ return service; }); }, - findInstanceBySlug: function(id, slug, dc, configuration) { + findInstanceBySlug: function(id, node, slug, dc, configuration) { return this.findBySlug(slug, dc, configuration).then(function(item) { + // Loop through all the service instances and pick out the one + // that has the same service id AND node name + // node names are unique per datacenter const i = item.Nodes.findIndex(function(item) { - return item.Service.ID === id; + return item.Service.ID === id && item.Node.Node === node; }); if (i !== -1) { const service = item.Nodes[i].Service; diff --git a/ui-v2/app/templates/dc/services/-instances.hbs b/ui-v2/app/templates/dc/services/-instances.hbs index 7ef34c340..9974aea02 100644 --- a/ui-v2/app/templates/dc/services/-instances.hbs +++ b/ui-v2/app/templates/dc/services/-instances.hbs @@ -19,7 +19,7 @@ {{/block-slot}} {{#block-slot 'row'}} - + {{ or item.Service.ID item.Service.Service }} diff --git a/ui-v2/tests/acceptance/dc/services/instances/error.feature b/ui-v2/tests/acceptance/dc/services/instances/error.feature index bf4ef1dc6..ae7e6bc76 100644 --- a/ui-v2/tests/acceptance/dc/services/instances/error.feature +++ b/ui-v2/tests/acceptance/dc/services/instances/error.feature @@ -7,9 +7,10 @@ Feature: dc / services / instances / error: Visit Service Instance what doesn't --- dc: dc1 service: service-0 + node: node-0 id: id-that-doesnt-exist --- - Then the url should be /dc1/services/service-0/id-that-doesnt-exist + Then the url should be /dc1/services/service-0/node-0/id-that-doesnt-exist And I see the text "404 (Unable to find instance)" in "[data-test-error]" diff --git a/ui-v2/tests/acceptance/dc/services/instances/proxy.feature b/ui-v2/tests/acceptance/dc/services/instances/proxy.feature index 1d69efb6b..4b2991a33 100644 --- a/ui-v2/tests/acceptance/dc/services/instances/proxy.feature +++ b/ui-v2/tests/acceptance/dc/services/instances/proxy.feature @@ -24,9 +24,10 @@ Feature: dc / services / instances / proxy: Show Proxy Service Instance --- dc: dc1 service: service-0-proxy + node: node-0 id: service-0-proxy-with-id --- - Then the url should be /dc1/services/service-0-proxy/service-0-proxy-with-id + Then the url should be /dc1/services/service-0-proxy/node-0/service-0-proxy-with-id And I see destination on the proxy like "service" And I see serviceChecksIsSelected on the tabs diff --git a/ui-v2/tests/acceptance/dc/services/instances/show.feature b/ui-v2/tests/acceptance/dc/services/instances/show.feature index d03d59f68..520660994 100644 --- a/ui-v2/tests/acceptance/dc/services/instances/show.feature +++ b/ui-v2/tests/acceptance/dc/services/instances/show.feature @@ -4,11 +4,19 @@ Feature: dc / services / instances / show: Show Service Instance Given 1 datacenter model with the value "dc1" And 1 service model from yaml --- + - Service: + ID: service-0-with-id + Meta: + external-source: consul + Node: + Node: node-0 - Service: ID: service-0-with-id Tags: ['Tag1', 'Tag2'] Meta: external-source: nomad + Node: + Node: node-1 Checks: - Name: Service check ServiceID: service-0 @@ -46,9 +54,10 @@ Feature: dc / services / instances / show: Show Service Instance --- dc: dc1 service: service-0 + node: node-1 id: service-0-with-id --- - Then the url should be /dc1/services/service-0/service-0-with-id + Then the url should be /dc1/services/service-0/node-1/service-0-with-id Then I don't see type on the proxy Then I see externalSource like "nomad" @@ -77,9 +86,10 @@ Feature: dc / services / instances / show: Show Service Instance --- dc: dc1 service: service-0 + node: node-0 id: service-0-with-id --- - Then the url should be /dc1/services/service-0/service-0-with-id + Then the url should be /dc1/services/service-0/node-0/service-0-with-id And an external edit results in 0 instance models And pause until I see the text "deregistered" in "[data-notification]" @ignore diff --git a/ui-v2/tests/acceptance/dc/services/instances/sidecar-proxy.feature b/ui-v2/tests/acceptance/dc/services/instances/sidecar-proxy.feature index 3a650565d..b26d103e4 100644 --- a/ui-v2/tests/acceptance/dc/services/instances/sidecar-proxy.feature +++ b/ui-v2/tests/acceptance/dc/services/instances/sidecar-proxy.feature @@ -16,9 +16,10 @@ Feature: dc / services / instances / sidecar-proxy: Show Sidecar Proxy Service I --- dc: dc1 service: service-0-sidecar-proxy + node: node-0 id: service-0-sidecar-proxy-with-id --- - Then the url should be /dc1/services/service-0-sidecar-proxy/service-0-sidecar-proxy-with-id + Then the url should be /dc1/services/service-0-sidecar-proxy/node-0/service-0-sidecar-proxy-with-id And I see destination on the proxy like "instance" And I see serviceChecksIsSelected on the tabs diff --git a/ui-v2/tests/acceptance/dc/services/instances/with-proxy.feature b/ui-v2/tests/acceptance/dc/services/instances/with-proxy.feature index 145bf95cd..d926c06a9 100644 --- a/ui-v2/tests/acceptance/dc/services/instances/with-proxy.feature +++ b/ui-v2/tests/acceptance/dc/services/instances/with-proxy.feature @@ -11,9 +11,10 @@ Feature: dc / services / instances / with-proxy: Show Service Instance with a pr --- dc: dc1 service: service-0 + node: node-0 id: service-0-with-id --- - Then the url should be /dc1/services/service-0/service-0-with-id + Then the url should be /dc1/services/service-0/node-0/service-0-with-id And I see type on the proxy like "proxy" And I see serviceChecksIsSelected on the tabs diff --git a/ui-v2/tests/acceptance/dc/services/instances/with-sidecar.feature b/ui-v2/tests/acceptance/dc/services/instances/with-sidecar.feature index 5e7c6c3f5..7f1fa32a2 100644 --- a/ui-v2/tests/acceptance/dc/services/instances/with-sidecar.feature +++ b/ui-v2/tests/acceptance/dc/services/instances/with-sidecar.feature @@ -11,9 +11,10 @@ Feature: dc / services / instances / with-sidecar: Show Service Instance with a --- dc: dc1 service: service-0 + node: node-0 id: service-0-with-id --- - Then the url should be /dc1/services/service-0/service-0-with-id + Then the url should be /dc1/services/service-0/node-0/service-0-with-id And I see type on the proxy like "sidecar-proxy" And I see serviceChecksIsSelected on the tabs diff --git a/ui-v2/tests/pages/dc/services/instance.js b/ui-v2/tests/pages/dc/services/instance.js index 69a707780..a728fad7f 100644 --- a/ui-v2/tests/pages/dc/services/instance.js +++ b/ui-v2/tests/pages/dc/services/instance.js @@ -1,6 +1,6 @@ export default function(visitable, attribute, collection, text, radiogroup) { return { - visit: visitable('/:dc/services/:service/:id'), + visit: visitable('/:dc/services/:service/:node/:id'), externalSource: attribute('data-test-external-source', 'h1 span'), tabs: radiogroup('tab', ['service-checks', 'node-checks', 'upstreams', 'tags']), serviceChecks: collection('#service-checks [data-test-healthchecks] li', {}),