ui: New search/sort/filtering bar for Node > ServiceInstances (#9326)

* Model layer changes to turn Node:ServiceInstances into hasMany

We tried to make something that feels a little like ember-data yet
not leave our approach of re-shaping the JSON directly from the
response.

1. We added transformHasManyResponse for re-shaping JSON for hasMany
relationships. we avoided the normalize word as ember-data serialize
methods usually return something JSON:API shaped and we distinctly don't
want to do that. Transform was the best word we could think of.

2. The integration tests across all of our models here feel very much
like those types of tests that aren't really testing much, or assert
too much to an extent that they get in the way rather than be of any
use. I'd very much like to move a lot of this to unit tests. Currently
most of the fingerprinting functionality is unit tested and these
integration tests were originally to give confidence that IDs and
related properties were being added correctly.

3. We've added a hasMany relationship, but not the corresponding
belongsTo - yet at least. We don't require the belongsTo right now, and
if we do  we can add it later.

* Integrate ServiceInstance search bar for Node:ServiceInstances

* Hide Node.Meta when on the Node:ServiceINstance page

We use a little string replace hack here for a human-like label, this is
soon to be replaced with proper i10n replacement

* Always ensure that a Namespace is set, and add comment explaining
This commit is contained in:
John Cowen 2020-12-09 13:08:30 +00:00 committed by GitHub
parent 4387740f01
commit 27c74f7141
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 250 additions and 217 deletions

View File

@ -17,11 +17,11 @@ import AdapterError, {
// the naming of things (serialized vs query etc)
const read = function(adapter, modelName, type, query = {}) {
return adapter.rpc(
function(adapter, request, query) {
return adapter[`requestFor${type}`](request, query);
function(adapter, ...rest) {
return adapter[`requestFor${type}`](...rest);
},
function(serializer, respond, query) {
return serializer[`respondFor${type}`](respond, query);
function(serializer, ...rest) {
return serializer[`respondFor${type}`](...rest);
},
query,
modelName
@ -29,11 +29,11 @@ const read = function(adapter, modelName, type, query = {}) {
};
const write = function(adapter, modelName, type, snapshot) {
return adapter.rpc(
function(adapter, request, serialized, unserialized) {
return adapter[`requestFor${type}`](request, serialized, unserialized);
function(adapter, ...rest) {
return adapter[`requestFor${type}`](...rest);
},
function(serializer, respond, serialized, unserialized) {
return serializer[`respondFor${type}`](respond, serialized, unserialized);
function(serializer, ...rest) {
return serializer[`respondFor${type}`](...rest);
},
snapshot,
modelName
@ -49,6 +49,7 @@ export default class HttpAdapter extends Adapter {
let unserialized, serialized;
const serializer = store.serializerFor(modelName);
const modelClass = store.modelFor(modelName);
// workable way to decide whether this is a snapshot
// essentially 'is attributable'.
// Snapshot is private so we can't do instanceof here
@ -66,14 +67,14 @@ export default class HttpAdapter extends Adapter {
return client
.request(function(request) {
return req(adapter, request, serialized, unserialized);
return req(adapter, request, serialized, unserialized, modelClass);
})
.catch(function(e) {
return adapter.error(e);
})
.then(function(respond) {
// TODO: When HTTPAdapter:responder changes, this will also need to change
return resp(serializer, respond, serialized, unserialized);
return resp(serializer, respond, serialized, unserialized, modelClass);
});
// TODO: Potentially add specific serializer errors here
// .catch(function(e) {

View File

@ -5,8 +5,8 @@
as |item index|>
<BlockSlot @name="header">
{{#if (eq @routeName "dc.services.show")}}
<a data-test-service-name href={{href-to @routeName item.Service}}>
{{item.ID}}
<a data-test-service-name href={{href-to @routeName item.Service.Service}}>
{{item.Service.ID}}
</a>
{{else}}
<a data-test-service-name href={{href-to @routeName item.Service.Service item.Node.Node (or item.Service.ID item.Service.Service)}}>
@ -15,9 +15,9 @@ as |item index|>
{{/if}}
</BlockSlot>
<BlockSlot @name="details">
{{#if @checks}}
<Consul::ExternalSource @item={{item}} />
<Consul::InstanceChecks @type="service" @items={{get @checks item.Service}} />
{{#if @node}}
<Consul::ExternalSource @item={{item.Service}} />
<Consul::InstanceChecks @type="service" @items={{item.Checks}} />
{{else}}
<Consul::ExternalSource @item={{item.Service}} />
<Consul::InstanceChecks @type="service" @items={{filter-by 'ServiceID' '' item.Checks}} />
@ -35,7 +35,7 @@ as |item index|>
</dd>
</dl>
{{/if}}
{{#if (gt item.Node.Node.length 0)}}
{{#if (not @node)}}
<dl class="node">
<dt>
<Tooltip>
@ -63,24 +63,6 @@ as |item index|>
</dd>
</dl>
{{/if}}
{{#if (and @checks item.Port)}}
<dl class="port">
<dt>
Port
</dt>
<dd data-test-service-port={{item.Port}}>
<CopyButton
@value={{item.Port}}
@name="Port"
/>
:{{item.Port}}
</dd>
</dl>
{{/if}}
{{#if checks}}
<TagList @item={{item}} />
{{else}}
<TagList @item={{item.Service}} />
{{/if}}
</BlockSlot>
</ListCollection>

View File

@ -21,13 +21,9 @@
</BlockSlot>
<BlockSlot @name="options">
{{#let components.Optgroup components.Option as |Optgroup Option|}}
<Option @value="Name" @selected={{contains 'Name' @filter.searchproperties}}>Name</Option>
<Option @value="Tags" @selected={{contains 'Tags' @filter.searchproperties}}>Tags</Option>
<Option @value="ID" @selected={{contains 'ID' @filter.searchproperties}}>Service ID</Option>
<Option @value="Address" @selected={{contains 'Address' @filter.searchproperties}}>Address</Option>
<Option @value="Port" @selected={{contains 'Port' @filter.searchproperties}}>Port</Option>
<Option @value="Service.Meta" @selected={{contains 'Service.Meta' @filter.searchproperties}}>Service Meta</Option>
<Option @value="Node.Meta" @selected={{contains 'Node.Meta' @filter.searchproperties}}>Node Meta</Option>
{{#each @searchproperties as |prop|}}
<Option @value={{prop}} @selected={{contains prop @filter.searchproperties}}>{{string-replace-all prop '\.' ' '}}</Option>
{{/each}}
{{/let}}
</BlockSlot>
</PopoverSelect>

View File

@ -1,28 +0,0 @@
import Controller from '@ember/controller';
import { get, computed } from '@ember/object';
export default class ServicesController extends Controller {
queryParams = {
search: {
as: 'filter',
replace: true,
},
};
@computed('item.Checks.[]')
get checks() {
const checks = {};
get(this, 'item.Checks')
.filter(item => {
return item.ServiceID !== '';
})
.forEach(item => {
if (typeof checks[item.ServiceID] === 'undefined') {
checks[item.ServiceID] = [];
}
checks[item.ServiceID].push(item);
});
return checks;
}
}

View File

@ -1,4 +1,4 @@
import Model, { attr } from '@ember-data/model';
import Model, { attr, hasMany } from '@ember-data/model';
import { computed } from '@ember/object';
import { fragmentArray } from 'ember-data-model-fragments/attributes';
@ -18,7 +18,7 @@ export default class Node extends Model {
@attr() meta; // {}
@attr() Meta; // {}
@attr() TaggedAddresses; // {lan, wan}
@attr() Services; // ServiceInstances[]
@hasMany('service-instance') Services; // TODO: Rename to ServiceInstances
@fragmentArray('health-check') Checks;
@computed('Checks.[]', 'ChecksCritical', 'ChecksPassing', 'ChecksWarning')

View File

@ -2,6 +2,13 @@ import Route from 'consul-ui/routing/route';
export default class ServicesRoute extends Route {
queryParams = {
sortBy: 'sort',
status: 'status',
source: 'source',
searchproperty: {
as: 'searchproperty',
empty: [['Name', 'Tags', 'ID', 'Address', 'Port', 'Service.Meta']],
},
search: {
as: 'filter',
replace: true,
@ -13,7 +20,10 @@ export default class ServicesRoute extends Route {
.split('.')
.slice(0, -1)
.join('.');
return this.modelFor(parent);
return {
...this.modelFor(parent),
searchProperties: this.queryParams.searchproperty.empty[0],
};
}
setupController(controller, model) {

View File

@ -20,7 +20,10 @@ export default class InstancesRoute extends Route {
.split('.')
.slice(0, -1)
.join('.');
return this.modelFor(parent);
return {
...this.modelFor(parent),
searchProperties: this.queryParams.searchproperty.empty[0],
};
}
setupController(controller, model) {

View File

@ -1,6 +1,14 @@
import Serializer from '@ember-data/serializer/rest';
export default class HttpSerializer extends Serializer {
transformBelongsToResponse(store, relationship, parent, item) {
return item;
}
transformHasManyResponse(store, relationship, parent, item) {
return item;
}
respondForQuery(respond, query) {
return respond((headers, body) => body);
}

View File

@ -1,5 +1,7 @@
import Serializer from './application';
import { EmbeddedRecordsMixin } from '@ember-data/serializer/rest';
import { PRIMARY_KEY, SLUG_KEY } from 'consul-ui/models/node';
import { classify } from '@ember/string';
// TODO: Looks like ID just isn't used at all consider just using .Node for
// the SLUG_KEY
@ -10,25 +12,70 @@ const fillSlug = function(item) {
return item;
};
export default class NodeSerializer extends Serializer {
export default class NodeSerializer extends Serializer.extend(EmbeddedRecordsMixin) {
primaryKey = PRIMARY_KEY;
slugKey = SLUG_KEY;
respondForQuery(respond, query) {
return super.respondForQuery(
attrs = {
Services: {
embedded: 'always',
},
};
transformHasManyResponse(store, relationship, item, parent = null) {
switch (relationship.key) {
case 'Services':
const checks = {};
(item.Checks || [])
.filter(item => {
return item.ServiceID !== '';
})
.forEach(item => {
if (typeof checks[item.ServiceID] === 'undefined') {
checks[item.ServiceID] = [];
}
checks[item.ServiceID].push(item);
});
const serializer = this.store.serializerFor(relationship.type);
item.Services = item.Services.map(service =>
serializer.transformHasManyResponseFromNode(item, service, checks)
);
return item;
}
return super.transformHasManyResponse(...arguments);
}
respondForQuery(respond, query, data, modelClass) {
const body = super.respondForQuery(
cb => respond((headers, body) => cb(headers, body.map(fillSlug))),
query
);
modelClass.eachRelationship((key, relationship) => {
body.forEach(item =>
this[`transform${classify(relationship.kind)}Response`](
this.store,
relationship,
item,
body
)
);
});
return body;
}
respondForQueryRecord(respond, query) {
return super.respondForQueryRecord(
respondForQueryRecord(respond, query, data, modelClass) {
const body = super.respondForQueryRecord(
cb =>
respond((headers, body) => {
return cb(headers, fillSlug(body));
}),
query
);
modelClass.eachRelationship((key, relationship) => {
this[`transform${classify(relationship.kind)}Response`](this.store, relationship, body);
});
return body;
}
respondForQueryLeader(respond, query) {

View File

@ -5,9 +5,62 @@ export default class ServiceInstanceSerializer extends Serializer {
primaryKey = PRIMARY_KEY;
slugKey = SLUG_KEY;
hash = JSON.stringify;
extractUid(item) {
return this.hash([
item.Namespace || 'default',
item.Datacenter,
item.Node.Node,
item.Service.ID,
]);
}
transformHasManyResponseFromNode(node, item, checks) {
const serviceChecks = checks[item.ID] || [];
const statuses = serviceChecks.reduce(
(prev, item) => {
switch (item.Status) {
case 'passing':
prev.ChecksPassing.push(item);
break;
case 'warning':
prev.ChecksWarning.push(item);
break;
case 'critical':
prev.ChecksCritical.push(item);
break;
}
return prev;
},
{
ChecksPassing: [],
ChecksWarning: [],
ChecksCritical: [],
}
);
const instance = {
...statuses,
Service: item,
Checks: serviceChecks,
Node: {
Datacenter: node.Datacenter,
Namespace: node.Namespace,
ID: node.ID,
Node: node.Node,
Address: node.Address,
TaggedAddresses: node.TaggedAddresses,
Meta: node.Meta,
},
};
instance.uid = this.extractUid(instance);
return instance;
}
respondForQuery(respond, query) {
return super.respondForQuery(function(cb) {
return respond(function(headers, body) {
const body = super.respondForQuery(cb => {
return respond((headers, body) => {
if (body.length === 0) {
const e = new Error();
e.errors = [
@ -18,14 +71,25 @@ export default class ServiceInstanceSerializer extends Serializer {
];
throw e;
}
body.forEach(item => {
item.Datacenter = query.dc;
item.Namespace = query.ns || 'default';
item.uid = this.extractUid(item);
});
return cb(headers, body);
});
}, query);
return body;
}
respondForQueryRecord(respond, query) {
return super.respondForQueryRecord(function(cb) {
return respond(function(headers, body) {
return super.respondForQueryRecord(cb => {
return respond((headers, body) => {
body.forEach(item => {
item.Datacenter = query.dc;
item.Namespace = query.ns || 'default';
item.uid = this.extractUid(item);
});
body = body.find(function(item) {
return item.Node.Node === query.node && item.Service.ID === query.serviceId;
});

View File

@ -1,19 +1,51 @@
{{#let item.Services as |items|}}
<div id="services" class="tab-section">
{{#let (hash
statuses=(if status (split status ',') undefined)
sources=(if source (split source ',') undefined)
searchproperties=(if (not-eq searchproperty undefined)
(split searchproperty ',')
searchProperties
)
) as |filters|}}
{{#let (or sortBy "Status:asc") as |sort|}}
{{#let item.Services as |items|}}
<div class="tab-section">
<div role="tabpanel">
{{#if (gt items.length 0) }}
<input type="checkbox" id="toolbar-toggle" />
<SearchBar
@placeholder="Search by name/port"
@value={{search}}
<Consul::ServiceInstance::SearchBar
@sources={{externalSources}}
@search={{search}}
@onsearch={{action (mut search) value="target.value"}}
@searchproperties={{searchProperties}}
@sort={{sort}}
@onsort={{action (mut sortBy) value="target.selected"}}
@filter={{filters}}
@onfilter={{hash
searchproperty=(action (mut searchproperty) value="target.selectedItems")
status=(action (mut status) value="target.selectedItems")
source=(action (mut source) value="target.selectedItems")
}}
/>
{{/if}}
<ChangeableSet @dispatcher={{searchable 'nodeservice' items}} @terms={{search}}>
<BlockSlot @name="set" as |filtered|>
<Consul::ServiceInstance::List @routeName="dc.services.show" @items={{filtered}} @checks={{checks}}/>
</BlockSlot>
<BlockSlot @name="empty">
{{! filter out any sidecar proxies }}
<DataCollection
@type="service-instance"
@sort={{sort}}
@filters={{filters}}
@search={{search}}
@items={{reject-by 'Service.Kind' 'connect-proxy' items}}
as |collection|>
<collection.Collection>
<Consul::ServiceInstance::List
@node={{item}}
@routeName="dc.services.show"
@items={{collection.items}}
@checks={{checks}}
/>
</collection.Collection>
<collection.Empty>
<EmptyState>
<BlockSlot @name="body">
<p>
@ -21,8 +53,10 @@
</p>
</BlockSlot>
</EmptyState>
</BlockSlot>
</ChangeableSet>
</collection.Empty>
</DataCollection>
</div>
</div>
{{/let}}
{{/let}}
{{/let}}

View File

@ -5,7 +5,7 @@
sources=(if source (split source ',') undefined)
searchproperties=(if (not-eq searchproperty undefined)
(split searchproperty ',')
(array 'Name' 'Tags' 'ID' 'Address' 'Port' 'Service.Meta' 'Node.Meta')
searchProperties
)
) as |filters|}}
{{#let (or sortBy "Status:asc") as |sort|}}
@ -15,6 +15,7 @@
@sources={{externalSources}}
@search={{search}}
@onsearch={{action (mut search) value="target.value"}}
@searchproperties={{searchProperties}}
@sort={{sort}}
@onsort={{action (mut sortBy) value="target.selected"}}
@ -27,6 +28,7 @@
}}
/>
{{/if}}
{{! Service > Service Instance view doesn't require filtering of proxies }}
<DataCollection
@type="service-instance"
@sort={{sort}}

View File

@ -13,14 +13,18 @@ export default function(foreignKey, nspaceKey, hash = JSON.stringify) {
return get(item, slugKey);
});
const nspaceValue = get(item, nspaceKey) || 'default';
return {
...item,
...{
[nspaceKey]: nspaceValue,
[foreignKey]: foreignKeyValue,
[primaryKey]: hash([nspaceValue, foreignKeyValue].concat(slugValues)),
},
};
// This ensures that all data objects have a Namespace value set, even
// in OSS. An empty Namespace will default to default
item[nspaceKey] = nspaceValue;
if (typeof item[foreignKey] === 'undefined') {
item[foreignKey] = foreignKeyValue;
}
if (typeof item[primaryKey] === 'undefined') {
item[primaryKey] = hash([nspaceValue, foreignKeyValue].concat(slugValues));
}
return item;
};
};
}

View File

@ -1,99 +0,0 @@
# TODO: If we keep separate types of catalog filters then
# these tests need splitting out, if we are moving nodes
# to use the name filter UI also, then they can stay together
@setupApplicationTest
Feature: components / catalog-filter
Scenario: Filtering [Model] in [Page]
Given 1 datacenter model with the value "dc1"
And 1 node model from yaml
---
ID: node-0
Services:
- ID: 'service-0-with-id'
Port: 65535
Service: 'service-0'
Tags: ['monitor', 'two', 'three']
- ID: 'service-1'
Port: 0
Service: 'service-1'
Tags: ['hard drive', 'monitor', 'three']
- ID: 'service-2'
Port: 1
Service: 'service-2'
Tags: ['one', 'two', 'three']
---
When I visit the [Page] page for yaml
---
dc: dc1
node: node-0
---
# And I see 3 healthcheck model with the name "Disk Util"
When I click serviceInstances on the tabs
And I see serviceInstancesIsSelected on the tabs
Then I fill in with yaml
---
s: 65535
---
And I see 1 [Model] model
And I see 1 [Model] model with the port "65535"
Then I fill in with yaml
---
s: hard drive
---
And I see 1 [Model] model with the name "[Model]-1"
Then I fill in with yaml
---
s: monitor
---
And I see 2 [Model] models
Then I fill in with yaml
---
s: wallpix
---
And I see 0 [Model] models
Where:
-------------------------------------------------
| Model | Page | Url |
| service | node | /dc-1/nodes/node-0 |
-------------------------------------------------
Scenario: Freetext filtering the service listing
Given 1 datacenter model with the value "dc-1"
And 3 service models from yaml
---
- Name: Service-0
Tags: ['one', 'two', 'three']
ChecksPassing: 0
ChecksWarning: 0
ChecksCritical: 1
Kind: ~
- Name: Service-1
Tags: ['two', 'three']
ChecksPassing: 0
ChecksWarning: 1
ChecksCritical: 0
Kind: ~
- Name: Service-2
Tags: ['three']
ChecksPassing: 1
ChecksWarning: 0
ChecksCritical: 0
Kind: ~
---
When I visit the services page for yaml
---
dc: dc-1
---
Then the url should be /dc-1/services
Then I see 3 service models
Then I fill in with yaml
---
s: three
---
And I see 3 service models
Then I fill in with yaml
---
s: 'tag:two'
---

View File

@ -10,21 +10,15 @@ module('Integration | Serializer | node', function(hooks) {
setupTest(hooks);
const nspace = 'default';
test('respondForQuery returns the correct data for list endpoint', function(assert) {
const store = this.owner.lookup('service:store');
const serializer = this.owner.lookup('serializer:node');
serializer.store = store;
const modelClass = store.modelFor('node');
const dc = 'dc-1';
const request = {
url: `/v1/internal/ui/nodes?dc=${dc}`,
};
return get(request.url).then(function(payload) {
const expected = payload.map(item =>
Object.assign({}, item, {
Datacenter: dc,
// TODO: default isn't required here, once we've
// refactored out our Serializer this can go
Namespace: nspace,
uid: `["${nspace}","${dc}","${item.ID}"]`,
})
);
const actual = serializer.respondForQuery(
function(cb) {
const headers = {};
@ -33,13 +27,22 @@ module('Integration | Serializer | node', function(hooks) {
},
{
dc: dc,
}
},
{
dc: dc,
},
modelClass
);
assert.deepEqual(actual, expected);
assert.equal(actual[0].Datacenter, dc);
assert.equal(actual[0].Namespace, nspace);
assert.equal(actual[0].uid, `["${nspace}","${dc}","${actual[0].ID}"]`);
});
});
test('respondForQueryRecord returns the correct data for item endpoint', function(assert) {
const store = this.owner.lookup('service:store');
const serializer = this.owner.lookup('serializer:node');
serializer.store = store;
const modelClass = store.modelFor('node');
const dc = 'dc-1';
const id = 'node-name';
const request = {
@ -65,9 +68,15 @@ module('Integration | Serializer | node', function(hooks) {
},
{
dc: dc,
}
},
{
dc: dc,
},
modelClass
);
assert.deepEqual(actual, expected);
assert.equal(actual.Datacenter, dc);
assert.equal(actual.Namespace, nspace);
assert.equal(actual.uid, `["${nspace}","${dc}","${actual.ID}"]`);
});
});
test('respondForQueryLeader returns the correct data', function(assert) {