From 59b044f96d416b77b90790ae21f5fa234dae2781 Mon Sep 17 00:00:00 2001 From: Michael Klein Date: Wed, 20 Jul 2022 17:07:52 +0200 Subject: [PATCH] ui: no partition and peer in bucket-list at the same time (#13812) * don't show partition / peer at the same time in bucket-list * use bucket-list in intentions table * add bucket-list tests * Simplify bucket list - match old behavior Refactor the bucket-list component to be easier to grok and match how the old template based approach worked. I.e. do not surface partition or namespace when it matches the passed nspace or partition property. * Update docs for bucket-list * fix linting --- .../components/consul/bucket/list/README.mdx | 5 +- .../components/consul/bucket/list/index.js | 131 +++++---- .../consul/intention/list/table/index.hbs | 43 +-- .../components/consul/bucket/list-test.js | 255 ++++++++++++++++++ 4 files changed, 344 insertions(+), 90 deletions(-) create mode 100644 ui/packages/consul-ui/tests/integration/components/consul/bucket/list-test.js diff --git a/ui/packages/consul-ui/app/components/consul/bucket/list/README.mdx b/ui/packages/consul-ui/app/components/consul/bucket/list/README.mdx index d6bdc01c6..39d1703b2 100644 --- a/ui/packages/consul-ui/app/components/consul/bucket/list/README.mdx +++ b/ui/packages/consul-ui/app/components/consul/bucket/list/README.mdx @@ -11,6 +11,9 @@ the namespace will be displayed, whereas if the partition is different it will show both the partition and namespace (as a namespace called 'team-1' in `partition-1` is different to a namespace called 'team-1' in `partition-2`) +When the passed item contains a `PeerName`, this will be displayed in place of +the `Partition`. + Showing the service name is a tiny bit awkward (different boolean type, doesn't care about difference) and could be improved but we only use it for the read only view of intentions. @@ -89,7 +92,7 @@ At the time of writing, this is not currently used across the entire UI | Argument/Attribute | Type | Default | Description | | --- | --- | --- | --- | -| `item` | `array` | | A Consul object that could have both a `Partition` and a `Namespace` property | +| `item` | `object` | | A Consul object that could have a `Partition`, a `Namespace`, a `PeerName` and a `Service` property | | `nspace` | `string` | | The name of the current namespace | | `partition` | `string` | | The name of the current partition | | `service` | `boolean` | | Whether to show the service name on the end or not. Please note you must also pass a item.Service for it to show. We flag this incase an API request has a Service property but you don't want to show it | diff --git a/ui/packages/consul-ui/app/components/consul/bucket/list/index.js b/ui/packages/consul-ui/app/components/consul/bucket/list/index.js index 9c48f1bd9..c874ac91b 100644 --- a/ui/packages/consul-ui/app/components/consul/bucket/list/index.js +++ b/ui/packages/consul-ui/app/components/consul/bucket/list/index.js @@ -5,84 +5,105 @@ export default class ConsulBucketList extends Component { @service abilities; get itemsToDisplay() { - const { item, partition, nspace } = this.args; - const { abilities } = this; + const { peerOrPartitionPart, namespacePart, servicePart } = this; - let items = []; + return [...peerOrPartitionPart, ...namespacePart, ...servicePart]; + } + + get peerOrPartitionPart() { + const { peerPart, partitionPart } = this; + + if (peerPart.length) { + return peerPart; + } else { + return partitionPart; + } + } + + get partitionPart() { + const { item, partition } = this.args; + + const { abilities } = this; if (partition && abilities.can('use partitions')) { if (item.Partition !== partition) { - this._addPeer(items); - this._addPartition(items); - this._addNamespace(items); - this._addService(items); - } else { - this._addPeerInfo(items); + return [ + { + type: 'partition', + label: 'Admin Partition', + item: item.Partition, + }, + ]; } - } else if (nspace && abilities.can('use nspace')) { - if (item.Namespace !== nspace) { - this._addPeerInfo(items); - this._addService(items); - } else { - this._addPeerInfo(items); - } - } else { - this._addPeerInfo(items); } - return items; + return []; } - _addPeerInfo(items) { + get peerPart() { const { item } = this.args; if (item.PeerName) { - this._addPeer(items); - this._addNamespace(items); + return [ + { + type: 'peer', + label: 'Peer', + item: item.PeerName, + }, + ]; } + + return []; } - _addPartition(items) { - const { item } = this.args; + get namespacePart() { + const { item, nspace } = this.args; + const { abilities, partitionPart } = this; - items.push({ - type: 'partition', - label: 'Admin Partition', - item: item.Partition, - }); - } - - _addNamespace(items) { - const { item } = this.args; - - items.push({ + const nspaceItem = { type: 'nspace', label: 'Namespace', item: item.Namespace, - }); + }; + + // when we surface a partition - show a namespace with it + if (partitionPart.length) { + return [nspaceItem]; + } + + if (nspace && abilities.can('use nspaces')) { + if (item.Namespace !== nspace) { + return [ + { + type: 'nspace', + label: 'Namespace', + item: item.Namespace, + }, + ]; + } + } + + return []; } - _addService(items) { - const { service, item } = this.args; + get servicePart() { + const { item, service } = this.args; - if (service && item.Service) { - items.push({ - type: 'service', - label: 'Service', - item: item.Service, - }); + const { partitionPart, namespacePart } = this; + + // when we show partitionPart or namespacePart -> consider service part + if (partitionPart.length || namespacePart.length) { + if (item.Service && service) { + return [ + { + type: 'service', + label: 'Service', + item: item.Service, + }, + ]; + } } - } - _addPeer(items) { - const { item } = this.args; - - if (item?.PeerName) { - items.push({ - type: 'peer', - label: 'Peer', - item: item.PeerName, - }); - } + return []; } } diff --git a/ui/packages/consul-ui/app/components/consul/intention/list/table/index.hbs b/ui/packages/consul-ui/app/components/consul/intention/list/table/index.hbs index 4bb110a89..03e47c17e 100644 --- a/ui/packages/consul-ui/app/components/consul/intention/list/table/index.hbs +++ b/ui/packages/consul-ui/app/components/consul/intention/list/table/index.hbs @@ -24,43 +24,18 @@ as |item index|> {{else}} {{item.SourceName}} {{/if}} - {{#if (or (can 'use nspaces') (can 'use partitions'))}} {{! TODO: slugify }} - {{#if item.SourcePeer}} - - - - - - - - - {{item.SourcePeer}} - - {{else}} - - {{or item.SourcePartition 'default'}} - - {{/if}} - / - {{or item.SourceNS 'default'}} + - {{/if}} diff --git a/ui/packages/consul-ui/tests/integration/components/consul/bucket/list-test.js b/ui/packages/consul-ui/tests/integration/components/consul/bucket/list-test.js new file mode 100644 index 000000000..15d6b81ce --- /dev/null +++ b/ui/packages/consul-ui/tests/integration/components/consul/bucket/list-test.js @@ -0,0 +1,255 @@ +import { module, test } from 'qunit'; +import { setupRenderingTest } from 'ember-qunit'; +import hbs from 'htmlbars-inline-precompile'; +import { render } from '@ember/test-helpers'; +import Service from '@ember/service'; + +module('Integration | Component | consul bucket list', function(hooks) { + setupRenderingTest(hooks); + + module('without nspace or partition feature on', function(hooks) { + hooks.beforeEach(function() { + this.owner.register( + 'service:abilities', + class Stub extends Service { + can(permission) { + if (permission === 'use partitions') { + return false; + } + if (permission === 'use nspaces') { + return false; + } + + return false; + } + } + ); + }); + + test('it displays a peer when the item passed has a peer name', async function(assert) { + const PEER_NAME = 'Tomster'; + + this.set('peerName', PEER_NAME); + + await render(hbs` + + `); + + assert.dom('[data-test-bucket-item="peer"]').hasText(PEER_NAME, 'Peer name is displayed'); + assert.dom('[data-test-bucket-item="nspace"]').doesNotExist('namespace is not shown'); + assert.dom('[data-test-bucket-item="partition"]').doesNotExist('partition is not shown'); + }); + + test('it does not display a bucket list when item has no peer name', async function(assert) { + await render(hbs` + + `); + + assert.dom('[data-test-bucket-list]').doesNotExist('no bucket list displayed'); + }); + }); + + module('with partition feature on', function(hooks) { + hooks.beforeEach(function() { + this.owner.register( + 'service:abilities', + class Stub extends Service { + can(permission) { + if (permission === 'use partitions') { + return true; + } + if (permission === 'use nspaces') { + return true; + } + + return false; + } + } + ); + }); + + test("it displays a peer and nspace and service and no partition when item.Partition and partition don't match", async function(assert) { + const PEER_NAME = 'Tomster'; + const NAMESPACE_NAME = 'Mascot'; + const SERVICE_NAME = 'Ember.js'; + + this.set('peerName', PEER_NAME); + this.set('namespace', NAMESPACE_NAME); + this.set('service', SERVICE_NAME); + + await render(hbs` + + `); + + assert.dom('[data-test-bucket-item="peer"]').hasText(PEER_NAME, 'Peer is displayed'); + assert + .dom('[data-test-bucket-item="nspace"]') + .hasText(NAMESPACE_NAME, 'namespace is displayed'); + assert.dom('[data-test-bucket-item="service"]').hasText(SERVICE_NAME, 'service is displayed'); + assert.dom('[data-test-bucket-item="partition"]').doesNotExist('partition is not displayed'); + }); + + test("it displays partition and nspace and service when item.Partition and partition don't match and peer is not set", async function(assert) { + const PARTITION_NAME = 'Ember.js'; + const NAMESPACE_NAME = 'Mascot'; + const SERVICE_NAME = 'Consul'; + + this.set('partition', PARTITION_NAME); + this.set('namespace', NAMESPACE_NAME); + this.set('service', SERVICE_NAME); + + await render(hbs` + + `); + + assert.dom('[data-test-bucket-item="peer"]').doesNotExist('peer is not displayed'); + assert + .dom('[data-test-bucket-item="nspace"]') + .hasText(NAMESPACE_NAME, 'namespace is displayed'); + assert.dom('[data-test-bucket-item="service"]').hasText(SERVICE_NAME, 'service is displayed'); + assert + .dom('[data-test-bucket-item="partition"]') + .hasText(PARTITION_NAME, 'partition is displayed'); + }); + + test('it displays nspace and peer and service when item.Partition and partition match and peer is set', async function(assert) { + const PEER_NAME = 'Tomster'; + const PARTITION_NAME = 'Ember.js'; + const NAMESPACE_NAME = 'Mascot'; + const SERVICE_NAME = 'Ember.js'; + + this.set('peerName', PEER_NAME); + this.set('partition', PARTITION_NAME); + this.set('namespace', NAMESPACE_NAME); + this.set('service', SERVICE_NAME); + + await render(hbs` + + `); + + assert.dom('[data-test-bucket-item="peer"]').hasText(PEER_NAME, 'peer is displayed'); + assert + .dom('[data-test-bucket-item="nspace"]') + .hasText(NAMESPACE_NAME, 'namespace is displayed'); + assert.dom('[data-test-bucket-item="service"]').hasText(SERVICE_NAME, 'service is displayed'); + assert.dom('[data-test-bucket-item="partition"]').doesNotExist('partition is not displayed'); + }); + }); + + module('with nspace on but partition feature off', function(hooks) { + hooks.beforeEach(function() { + this.owner.register( + 'service:abilities', + class Stub extends Service { + can(permission) { + if (permission === 'use partitions') { + return false; + } + if (permission === 'use nspaces') { + return true; + } + + return false; + } + } + ); + }); + + test("it displays a peer and nspace and service when item.namespace and nspace don't match", async function(assert) { + const PEER_NAME = 'Tomster'; + const NAMESPACE_NAME = 'Mascot'; + const SERVICE_NAME = 'Ember.js'; + + this.set('peerName', PEER_NAME); + this.set('namespace', NAMESPACE_NAME); + this.set('service', SERVICE_NAME); + + await render(hbs` + + `); + + assert.dom('[data-test-bucket-item="peer"]').hasText(PEER_NAME, 'Peer is displayed'); + assert + .dom('[data-test-bucket-item="nspace"]') + .hasText(NAMESPACE_NAME, 'namespace is displayed'); + assert.dom('[data-test-bucket-item="service"]').hasText(SERVICE_NAME, 'service is displayed'); + assert.dom('[data-test-bucket-item="partition"]').doesNotExist('partition is not displayed'); + }); + + test('it displays a peer and no nspace and no service when item.namespace and nspace match', async function(assert) { + const PEER_NAME = 'Tomster'; + const NAMESPACE_NAME = 'Mascot'; + const SERVICE_NAME = 'Ember.js'; + + this.set('peerName', PEER_NAME); + this.set('namespace', NAMESPACE_NAME); + this.set('service', SERVICE_NAME); + + await render(hbs` + + `); + + assert.dom('[data-test-bucket-item="peer"]').hasText(PEER_NAME, 'Peer is displayed'); + assert.dom('[data-test-bucket-item="nspace"]').doesNotExist('namespace is not displayed'); + assert.dom('[data-test-bucket-item="service"]').doesNotExist('service is not displayed'); + assert.dom('[data-test-bucket-item="partition"]').doesNotExist('partition is not displayed'); + }); + }); +});