ui: Adds blocking query support to the service detail page (#5479)

This commit includes several pieces of functionality to enable services
to be removed and the page to present information that this has happened
but also keep the deleted information on the page. Along with the more
usual blocking query based listing.

To enable this:

1. Implements `meta` on the model (only available on collections in
ember)
2. Adds new `catchable` ComputedProperty alongside a `listen` helper for
working with specific errors that can be thrown from EventSources in an
ember-like way. Briefly, normal computed properties update when a
property changes, EventSources can additionally throw errors so we can
catch them and show different visuals based on that.
This commit is contained in:
John Cowen 2019-03-22 17:10:33 +00:00 committed by John Cowen
parent 45a2fa5a01
commit 1625a09372
17 changed files with 144 additions and 31 deletions

View File

@ -0,0 +1,11 @@
import ComputedProperty from '@ember/object/computed';
import computedFactory from 'consul-ui/utils/computed/factory';
export default class Catchable extends ComputedProperty {
catch(cb) {
return this.meta({
catch: cb,
});
}
}
export const computed = computedFactory(Catchable);

View File

@ -1,9 +1,13 @@
import Controller from '@ember/controller';
import { get, set, computed } from '@ember/object';
import { alias } from '@ember/object/computed';
import { inject as service } from '@ember/service';
import WithSearching from 'consul-ui/mixins/with-searching';
export default Controller.extend(WithSearching, {
import WithEventSource, { listen } from 'consul-ui/mixins/with-event-source';
export default Controller.extend(WithEventSource, WithSearching, {
dom: service('dom'),
notify: service('flashMessages'),
items: alias('item.Nodes'),
init: function() {
this.searchParams = {
serviceInstance: 's',
@ -17,6 +21,19 @@ export default Controller.extend(WithSearching, {
// need this variable
set(this, 'selectedTab', 'instances');
},
item: listen('item').catch(function(e) {
if (e.target.readyState === 1) {
// OPEN
if (get(e, 'error.errors.firstObject.status') === '404') {
get(this, 'notify').add({
destroyOnClick: false,
sticky: true,
type: 'warning',
action: 'update',
});
}
}
}),
searchable: computed('items', function() {
return get(this, 'searchables.serviceInstance')
.add(get(this, 'items'))

View File

@ -34,6 +34,12 @@ export function initialize(container) {
repo: 'repository/service/event-source',
},
},
{
route: 'dc/services/show',
services: {
repo: 'repository/service/event-source',
},
},
])
.forEach(function(definition) {
if (typeof definition.extend !== 'undefined') {

View File

@ -1,12 +1,38 @@
import Mixin from '@ember/object/mixin';
import { computed as catchable } from 'consul-ui/computed/catchable';
import purify from 'consul-ui/utils/computed/purify';
export default Mixin.create({
import WithListeners from 'consul-ui/mixins/with-listeners';
const PREFIX = '_';
export default Mixin.create(WithListeners, {
setProperties: function(model) {
const _model = {};
Object.keys(model).forEach(key => {
// here (see comment below on deleting)
if (typeof this[key] !== 'undefined' && this[key].isDescriptor) {
_model[`${PREFIX}${key}`] = model[key];
const meta = this.constructor.metaForProperty(key) || {};
if (typeof meta.catch === 'function') {
if (typeof _model[`${PREFIX}${key}`].addEventListener === 'function') {
this.listen(_model[`_${key}`], 'error', meta.catch.bind(this));
}
}
} else {
_model[key] = model[key];
}
});
return this._super(_model);
},
reset: function(exiting) {
if (exiting) {
Object.keys(this).forEach(prop => {
if (this[prop] && typeof this[prop].close === 'function') {
this[prop].close();
// ember doesn't delete on 'resetController' by default
// right now we only call reset when we are exiting, therefore a full
// setProperties will be called the next time we enter the Route so this
// is ok for what we need and means that the above conditional works
// as expected (see 'here' comment above)
delete this[prop];
}
});
@ -14,3 +40,6 @@ export default Mixin.create({
return this._super(...arguments);
},
});
export const listen = purify(catchable, function(props) {
return props.map(item => `${PREFIX}${item}`);
});

View File

@ -30,6 +30,7 @@ export default Model.extend({
Node: attr(),
Service: attr(),
Checks: attr(),
meta: attr(),
passing: computed('ChecksPassing', 'Checks', function() {
let num = 0;
// TODO: use typeof

View File

@ -15,14 +15,6 @@ export default Route.extend({
const repo = get(this, 'repo');
return hash({
item: repo.findBySlug(params.name, this.modelFor('dc').dc.Name),
}).then(function(model) {
return {
...model,
...{
// Nodes happen to be the ServiceInstances here
items: model.item.Nodes,
},
};
});
},
setupController: function(controller, model) {

View File

@ -15,15 +15,6 @@ export default Serializer.extend({
const headers = payload[HTTP_HEADERS_SYMBOL] || {};
delete payload[HTTP_HEADERS_SYMBOL];
const normalizedPayload = this.normalizePayload(payload, id, requestType);
const response = this._super(
store,
primaryModelClass,
{
[primaryModelClass.modelName]: normalizedPayload,
},
id,
requestType
);
// put the meta onto the response, here this is ok
// as JSON-API allows this and our specific data is now in
// response[primaryModelClass.modelName]
@ -31,7 +22,7 @@ export default Serializer.extend({
// (which was the reason for the Symbol-like property earlier)
// use a method modelled on ember-data methods so we have the opportunity to
// do this on a per-model level
response.meta = this.normalizeMeta(
const meta = this.normalizeMeta(
store,
primaryModelClass,
headers,
@ -39,7 +30,19 @@ export default Serializer.extend({
id,
requestType
);
return response;
if (requestType === 'queryRecord') {
normalizedPayload.meta = meta;
}
return this._super(
store,
primaryModelClass,
{
meta: meta,
[primaryModelClass.modelName]: normalizedPayload,
},
id,
requestType
);
},
normalizeMeta: function(store, primaryModelClass, headers, payload, id, requestType) {
const meta = {

View File

@ -8,6 +8,18 @@ export default RepositoryService.extend({
findBySlug: function(slug, dc) {
return this._super(...arguments).then(function(item) {
const nodes = get(item, 'Nodes');
if (nodes.length === 0) {
// TODO: Add an store.error("404", "message") or similar
// or move all this to serializer
const e = new Error();
e.errors = [
{
status: '404',
title: 'Not found',
},
];
throw e;
}
const service = get(nodes, 'firstObject');
const tags = nodes
.reduce(function(prev, item) {
@ -16,6 +28,7 @@ export default RepositoryService.extend({
.uniq();
set(service, 'Tags', tags);
set(service, 'Nodes', nodes);
set(service, 'meta', get(item, 'meta'));
return service;
});
},
@ -36,6 +49,7 @@ export default RepositoryService.extend({
return service;
}
// TODO: Add an store.error("404", "message") or similar
// or move all this to serializer
const e = new Error();
e.errors = [
{

View File

@ -0,0 +1,7 @@
{{#if (eq type 'update')}}
{{#if (eq status 'warning') }}
This service has been deregistered and no longer exists in the catalog.
{{else}}
{{/if}}
{{/if}}

View File

@ -1,7 +1,6 @@
{{#app-view class="service list"}}
{{!TODO: Look at the item passed through to figure what partial to show, also move into its own service partial, for the moment keeping here for visibility}}
{{#block-slot 'notification' as |status type|}}
{{partial 'dc/acls/notifications'}}
{{partial 'dc/services/notifications'}}
{{/block-slot}}
{{#block-slot 'header'}}
<h1>

View File

@ -1,4 +1,7 @@
{{#app-view class="service show"}}
{{#block-slot 'notification' as |status type|}}
{{partial 'dc/services/notifications'}}
{{/block-slot}}
{{#block-slot 'breadcrumbs'}}
<ol>
<li><a data-test-back href={{href-to 'dc.services'}}>All Services</a></li>

View File

@ -27,7 +27,9 @@ module.exports = function(environment) {
injectionFactories: ['view', 'controller', 'component'],
},
};
// TODO: These should probably go onto APP
ENV = Object.assign({}, ENV, {
CONSUL_UI_DISABLE_REALTIME: false,
CONSUL_GIT_SHA: (function() {
if (process.env.CONSUL_GIT_SHA) {
return process.env.CONSUL_GIT_SHA;

View File

@ -10,8 +10,8 @@ Feature: dc / list-blocking
consul:client:
blocking: 1
---
Scenario:
And 3 [Model] models
Scenario: Viewing the listing pages
Given 3 [Model] models
And a network latency of 100
When I visit the [Page] page for yaml
---
@ -26,8 +26,31 @@ Feature: dc / list-blocking
And an external edit results in 0 [Model] models
And pause until I see 0 [Model] models
Where:
--------------------------------------------
| Page | Model | Url |
| services | service | services |
| nodes | node | nodes |
--------------------------------------------
------------------------------------------------
| Page | Model | Url |
| services | service | services |
| nodes | node | nodes |
------------------------------------------------
Scenario: Viewing detail pages with a listing
Given 3 [Model] models
And a network latency of 100
When I visit the [Page] page for yaml
---
dc: dc-1
service: service-0
---
Then the url should be /dc-1/[Url]
And pause until I see 3 [Model] models
And an external edit results in 5 [Model] models
And pause until I see 5 [Model] models
And an external edit results in 1 [Model] model
And pause until I see 1 [Model] model
And an external edit results in 0 [Model] models
And pause for 300
And I see the text "deregistered" in "[data-notification]"
Where:
------------------------------------------------
| Page | Model | Url |
| service | instance | services/service-0 |
------------------------------------------------

View File

@ -9,6 +9,7 @@ export default function(type, count, obj) {
key = 'CONSUL_SERVICE_COUNT';
break;
case 'node':
case 'instance':
key = 'CONSUL_NODE_COUNT';
break;
case 'kv':

View File

@ -5,6 +5,7 @@ export default function(type) {
requests = ['/v1/catalog/datacenters'];
break;
case 'service':
case 'instance':
requests = ['/v1/internal/ui/services', '/v1/health/service/'];
break;
case 'proxy':

View File

@ -68,6 +68,10 @@ test('findBySlug returns the correct data for item endpoint', function(assert) {
const service = payload.Nodes[0];
service.Nodes = nodes;
service.Tags = payload.Nodes[0].Service.Tags;
service.meta = {
date: undefined,
cursor: undefined,
};
return service;
})

View File

@ -2,7 +2,7 @@ import { moduleFor, test } from 'ember-qunit';
moduleFor('controller:dc/services/show', 'Unit | Controller | dc/services/show', {
// Specify the other units that are required for this test.
needs: ['service:search', 'service:dom'],
needs: ['service:search', 'service:dom', 'service:flashMessages'],
});
// Replace this with your real tests.