ocket8888 commented on code in PR #7615: URL: https://github.com/apache/trafficcontrol/pull/7615#discussion_r1265609344
########## experimental/traffic-portal/src/app/api/testing/topology.service.ts: ########## @@ -0,0 +1,130 @@ +/* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ + +import { Injectable } from "@angular/core"; +import { + RequestTopology, + ResponseTopology, + ResponseTopologyNode +} from "trafficops-types"; + +/** + * TopologyService expose API functionality relating to Topologies. + */ +@Injectable() +export class TopologyService { + private readonly topologies: ResponseTopology[] = [ + { + description: "", + lastUpdated: new Date(), + name: "test", + nodes: [ + { + cachegroup: "Edge", + parents: [1], + }, + { + cachegroup: "Mid", + parents: [2], + }, + { + cachegroup: "Origin", + parents: [], + }, + ], + } + ]; + + /** + * Gets one or all Topologies from Traffic Ops + * + * @param name? The integral, unique identifier of a single Topology to be Review Comment: I think this is a valid JSDoc, but you don't need to put the `?` in the param name. It's not a linter error, so I'm fine with this if you want to keep it, but it's not done anywhere else, and the compiler keeps track of what is actually optional or not without that needing to be spelled out in the comment. ########## experimental/traffic-portal/src/app/api/testing/topology.service.ts: ########## @@ -0,0 +1,130 @@ +/* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ + +import { Injectable } from "@angular/core"; +import { + RequestTopology, + ResponseTopology, + ResponseTopologyNode +} from "trafficops-types"; + +/** + * TopologyService expose API functionality relating to Topologies. + */ +@Injectable() +export class TopologyService { + private readonly topologies: ResponseTopology[] = [ + { + description: "", + lastUpdated: new Date(), + name: "test", + nodes: [ + { + cachegroup: "Edge", + parents: [1], + }, + { + cachegroup: "Mid", + parents: [2], + }, + { + cachegroup: "Origin", + parents: [], + }, + ], + } + ]; + + /** + * Gets one or all Topologies from Traffic Ops + * + * @param name? The integral, unique identifier of a single Topology to be + * returned + * @returns Either a Map of Topology names to full Topology objects, or a single Topology, depending on whether `id` was Review Comment: This whole `returns` directive looks wrong for several reasons ########## experimental/traffic-portal/src/app/api/testing/topology.service.ts: ########## @@ -0,0 +1,130 @@ +/* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ + +import { Injectable } from "@angular/core"; +import { + RequestTopology, + ResponseTopology, + ResponseTopologyNode +} from "trafficops-types"; + +/** + * TopologyService expose API functionality relating to Topologies. + */ +@Injectable() +export class TopologyService { + private readonly topologies: ResponseTopology[] = [ + { + description: "", + lastUpdated: new Date(), + name: "test", + nodes: [ + { + cachegroup: "Edge", + parents: [1], + }, + { + cachegroup: "Mid", + parents: [2], + }, + { + cachegroup: "Origin", + parents: [], + }, + ], + } + ]; + + /** + * Gets one or all Topologies from Traffic Ops + * + * @param name? The integral, unique identifier of a single Topology to be + * returned + * @returns Either a Map of Topology names to full Topology objects, or a single Topology, depending on whether `id` was + * passed. + * (In the event that `id` is passed but does not match any Topology, `null` will be emitted) + */ + public async getTopologies(name?: string): Promise<Array<ResponseTopology> | ResponseTopology> { + if (name !== undefined) { + const topology = this.topologies.find(t => t.name === name); + if (!topology) { + throw new Error(`no such Topology #${name}`); + } + return topology; + } + return this.topologies; + } + + /** + * Deletes a Topology. + * + * @param topology The Topology to be deleted, or just its ID. Review Comment: Topologies don't have IDs ########## experimental/traffic-portal/src/app/api/topology.service.ts: ########## @@ -0,0 +1,196 @@ +/* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ +import { HttpClient } from "@angular/common/http"; +import { Injectable } from "@angular/core"; +import type { + RequestTopology, + ResponseTopology, + ResponseTopologyNode, +} from "trafficops-types"; + +import { APIService } from "./base-api.service"; + +/** + * TopTreeNode is used to represent a topology in a format usable as a material + * nested tree data source. + */ +export interface TopTreeNode { + name: string; + cachegroup: string; + children: Array<TopTreeNode>; + parents: Array<this>; +} + +/** + * TopologyService exposes API functionality relating to Topologies. + */ +@Injectable() +export class TopologyService extends APIService { + + constructor(http: HttpClient) { + super(http); + } + + /** + * Gets a specific Topology from Traffic Ops + * + * @param name The name of the Topology to be returned. + * @returns The Topology with the given name. + */ + public async getTopologies(name: string): Promise<ResponseTopology>; + /** + * Gets all Topologies from Traffic Ops + * + * @returns An Array of all Topologies configured in Traffic Ops. + */ + public async getTopologies(): Promise<Array<ResponseTopology>>; + /** + * Gets one or all Topologies from Traffic Ops + * + * @param name The name of a single Topology to be returned. + * @returns Either an Array of Topology objects, or a single Topology, depending on + * whether `name` was passed. + */ + public async getTopologies(name?: string): Promise<Array<ResponseTopology> | ResponseTopology> { + const path = "topologies"; + if (name) { + const topology = await this.get<[ResponseTopology]>(path, undefined, {name}).toPromise(); + if (topology.length !== 1) { + throw new Error(`${topology.length} Topologies found by name ${name}`); + } + return topology[0]; + } + return this.get<Array<ResponseTopology>>(path).toPromise(); + } + + /** + * Deletes a Topology. + * + * @param topology The Topology to be deleted, or just its name. + */ + public async deleteTopology(topology: ResponseTopology | string): Promise<void> { + const name = typeof topology === "string" ? topology : topology.name; + return this.delete(`topologies?name=${name}`).toPromise(); + } + + /** + * Creates a new Topology. + * + * @param topology The Topology to create. + */ + public async createTopology(topology: RequestTopology): Promise<ResponseTopology> { + return this.post<ResponseTopology>("topologies", topology).toPromise(); + } + + /** + * Replaces an existing Topology with the provided new definition of a + * Topology. + * + * @param topology The full new definition of the Topology being updated + */ + public async updateTopology(topology: ResponseTopology): Promise<ResponseTopology> { + return this.put<ResponseTopology>(`topologies?name=${topology.name}`, topology).toPromise(); + } + + /** + * Generates a material tree from a topology. + * + * @param topology The topology to generate a material tree from. + * @returns a material tree. + */ + public topologyToTree(topology: ResponseTopology): Array<TopTreeNode> { + const treeNodes: Array<TopTreeNode> = []; + const topLevel: Array<TopTreeNode> = []; + for (const node of topology.nodes) { + const name = node.cachegroup; + const cachegroup = node.cachegroup; + const children: Array<TopTreeNode> = []; + const parents: Array<TopTreeNode> = []; Review Comment: Seems like it'd be faster to just put these in the object immediately, since they aren't being used anywhere else ########## experimental/traffic-portal/src/app/api/topology.service.ts: ########## @@ -0,0 +1,196 @@ +/* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ +import { HttpClient } from "@angular/common/http"; +import { Injectable } from "@angular/core"; +import type { + RequestTopology, + ResponseTopology, + ResponseTopologyNode, +} from "trafficops-types"; + +import { APIService } from "./base-api.service"; + +/** + * TopTreeNode is used to represent a topology in a format usable as a material + * nested tree data source. + */ +export interface TopTreeNode { + name: string; + cachegroup: string; + children: Array<TopTreeNode>; + parents: Array<this>; +} + +/** + * TopologyService exposes API functionality relating to Topologies. + */ +@Injectable() +export class TopologyService extends APIService { + + constructor(http: HttpClient) { + super(http); + } + + /** + * Gets a specific Topology from Traffic Ops + * + * @param name The name of the Topology to be returned. + * @returns The Topology with the given name. + */ + public async getTopologies(name: string): Promise<ResponseTopology>; + /** + * Gets all Topologies from Traffic Ops + * + * @returns An Array of all Topologies configured in Traffic Ops. + */ + public async getTopologies(): Promise<Array<ResponseTopology>>; + /** + * Gets one or all Topologies from Traffic Ops + * + * @param name The name of a single Topology to be returned. + * @returns Either an Array of Topology objects, or a single Topology, depending on + * whether `name` was passed. + */ + public async getTopologies(name?: string): Promise<Array<ResponseTopology> | ResponseTopology> { + const path = "topologies"; + if (name) { + const topology = await this.get<[ResponseTopology]>(path, undefined, {name}).toPromise(); + if (topology.length !== 1) { + throw new Error(`${topology.length} Topologies found by name ${name}`); + } + return topology[0]; + } + return this.get<Array<ResponseTopology>>(path).toPromise(); + } + + /** + * Deletes a Topology. + * + * @param topology The Topology to be deleted, or just its name. + */ + public async deleteTopology(topology: ResponseTopology | string): Promise<void> { + const name = typeof topology === "string" ? topology : topology.name; + return this.delete(`topologies?name=${name}`).toPromise(); + } + + /** + * Creates a new Topology. + * + * @param topology The Topology to create. + */ + public async createTopology(topology: RequestTopology): Promise<ResponseTopology> { + return this.post<ResponseTopology>("topologies", topology).toPromise(); + } + + /** + * Replaces an existing Topology with the provided new definition of a + * Topology. + * + * @param topology The full new definition of the Topology being updated + */ + public async updateTopology(topology: ResponseTopology): Promise<ResponseTopology> { Review Comment: Are Topology names immutable? If they aren't, then this method is incapable of modifying them, because it only allows passing one name. ########## experimental/traffic-portal/src/app/api/topology.service.ts: ########## @@ -0,0 +1,196 @@ +/* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ +import { HttpClient } from "@angular/common/http"; +import { Injectable } from "@angular/core"; +import type { + RequestTopology, + ResponseTopology, + ResponseTopologyNode, +} from "trafficops-types"; + +import { APIService } from "./base-api.service"; + +/** + * TopTreeNode is used to represent a topology in a format usable as a material + * nested tree data source. + */ +export interface TopTreeNode { + name: string; + cachegroup: string; + children: Array<TopTreeNode>; + parents: Array<this>; +} + +/** + * TopologyService exposes API functionality relating to Topologies. + */ +@Injectable() +export class TopologyService extends APIService { + + constructor(http: HttpClient) { + super(http); + } + + /** + * Gets a specific Topology from Traffic Ops + * + * @param name The name of the Topology to be returned. + * @returns The Topology with the given name. + */ + public async getTopologies(name: string): Promise<ResponseTopology>; + /** + * Gets all Topologies from Traffic Ops + * + * @returns An Array of all Topologies configured in Traffic Ops. + */ + public async getTopologies(): Promise<Array<ResponseTopology>>; + /** + * Gets one or all Topologies from Traffic Ops + * + * @param name The name of a single Topology to be returned. + * @returns Either an Array of Topology objects, or a single Topology, depending on + * whether `name` was passed. + */ + public async getTopologies(name?: string): Promise<Array<ResponseTopology> | ResponseTopology> { + const path = "topologies"; + if (name) { + const topology = await this.get<[ResponseTopology]>(path, undefined, {name}).toPromise(); + if (topology.length !== 1) { + throw new Error(`${topology.length} Topologies found by name ${name}`); + } + return topology[0]; + } + return this.get<Array<ResponseTopology>>(path).toPromise(); + } + + /** + * Deletes a Topology. + * + * @param topology The Topology to be deleted, or just its name. + */ + public async deleteTopology(topology: ResponseTopology | string): Promise<void> { + const name = typeof topology === "string" ? topology : topology.name; + return this.delete(`topologies?name=${name}`).toPromise(); + } + + /** + * Creates a new Topology. + * + * @param topology The Topology to create. + */ + public async createTopology(topology: RequestTopology): Promise<ResponseTopology> { + return this.post<ResponseTopology>("topologies", topology).toPromise(); + } + + /** + * Replaces an existing Topology with the provided new definition of a + * Topology. + * + * @param topology The full new definition of the Topology being updated + */ + public async updateTopology(topology: ResponseTopology): Promise<ResponseTopology> { + return this.put<ResponseTopology>(`topologies?name=${topology.name}`, topology).toPromise(); + } + + /** + * Generates a material tree from a topology. + * + * @param topology The topology to generate a material tree from. + * @returns a material tree. + */ + public topologyToTree(topology: ResponseTopology): Array<TopTreeNode> { + const treeNodes: Array<TopTreeNode> = []; + const topLevel: Array<TopTreeNode> = []; + for (const node of topology.nodes) { + const name = node.cachegroup; + const cachegroup = node.cachegroup; + const children: Array<TopTreeNode> = []; + const parents: Array<TopTreeNode> = []; + treeNodes.push({ + cachegroup, + children, + name, + parents, + }); + } + for (let index = 0; index < topology.nodes.length; index++) { + const node = topology.nodes[index]; + const treeNode = treeNodes[index]; + if (!(node.parents instanceof Array) || node.parents.length < 1) { + topLevel.push(treeNode); + continue; + } + for (const parent of node.parents) { + treeNodes[parent].children.push(treeNode); + treeNode.parents.push(treeNodes[parent]); + } + } + return topLevel; + } + + /** + * Generates a topology from a material tree. + * + * @param name The topology name + * @param description The topology description + * @param treeNodes The data for a material tree + * @returns a material tree. + */ + public treeToTopology(name: string, description: string, treeNodes: Array<TopTreeNode>): ResponseTopology { Review Comment: This references "material tree"s a lot, but doesn't actually appear to do anything with something by that name ########## experimental/traffic-portal/src/app/api/testing/topology.service.ts: ########## @@ -0,0 +1,130 @@ +/* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ + +import { Injectable } from "@angular/core"; +import { + RequestTopology, + ResponseTopology, + ResponseTopologyNode +} from "trafficops-types"; + +/** + * TopologyService expose API functionality relating to Topologies. + */ +@Injectable() +export class TopologyService { + private readonly topologies: ResponseTopology[] = [ + { + description: "", + lastUpdated: new Date(), + name: "test", + nodes: [ + { + cachegroup: "Edge", + parents: [1], + }, + { + cachegroup: "Mid", + parents: [2], + }, + { + cachegroup: "Origin", + parents: [], + }, + ], + } + ]; + + /** + * Gets one or all Topologies from Traffic Ops + * + * @param name? The integral, unique identifier of a single Topology to be + * returned + * @returns Either a Map of Topology names to full Topology objects, or a single Topology, depending on whether `id` was + * passed. + * (In the event that `id` is passed but does not match any Topology, `null` will be emitted) + */ + public async getTopologies(name?: string): Promise<Array<ResponseTopology> | ResponseTopology> { + if (name !== undefined) { + const topology = this.topologies.find(t => t.name === name); + if (!topology) { + throw new Error(`no such Topology #${name}`); + } + return topology; + } + return this.topologies; + } + + /** + * Deletes a Topology. + * + * @param topology The Topology to be deleted, or just its ID. + */ + public async deleteTopology(topology: ResponseTopology | string): Promise<void> { + const name = typeof topology === "string" ? topology : topology.name; + const idx = this.topologies.findIndex(t => t.name === name); + if (idx < 0) { + throw new Error(`no such Topology: #${name}`); Review Comment: Same as above ########## experimental/traffic-portal/src/app/api/topology.service.ts: ########## @@ -0,0 +1,196 @@ +/* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ +import { HttpClient } from "@angular/common/http"; +import { Injectable } from "@angular/core"; +import type { + RequestTopology, + ResponseTopology, + ResponseTopologyNode, +} from "trafficops-types"; + +import { APIService } from "./base-api.service"; + +/** + * TopTreeNode is used to represent a topology in a format usable as a material + * nested tree data source. + */ +export interface TopTreeNode { + name: string; + cachegroup: string; + children: Array<TopTreeNode>; + parents: Array<this>; +} + +/** + * TopologyService exposes API functionality relating to Topologies. + */ +@Injectable() +export class TopologyService extends APIService { + + constructor(http: HttpClient) { + super(http); + } + + /** + * Gets a specific Topology from Traffic Ops + * + * @param name The name of the Topology to be returned. + * @returns The Topology with the given name. + */ + public async getTopologies(name: string): Promise<ResponseTopology>; + /** + * Gets all Topologies from Traffic Ops + * + * @returns An Array of all Topologies configured in Traffic Ops. + */ + public async getTopologies(): Promise<Array<ResponseTopology>>; + /** + * Gets one or all Topologies from Traffic Ops + * + * @param name The name of a single Topology to be returned. + * @returns Either an Array of Topology objects, or a single Topology, depending on + * whether `name` was passed. + */ + public async getTopologies(name?: string): Promise<Array<ResponseTopology> | ResponseTopology> { + const path = "topologies"; + if (name) { + const topology = await this.get<[ResponseTopology]>(path, undefined, {name}).toPromise(); + if (topology.length !== 1) { + throw new Error(`${topology.length} Topologies found by name ${name}`); + } + return topology[0]; + } + return this.get<Array<ResponseTopology>>(path).toPromise(); + } + + /** + * Deletes a Topology. + * + * @param topology The Topology to be deleted, or just its name. + */ + public async deleteTopology(topology: ResponseTopology | string): Promise<void> { + const name = typeof topology === "string" ? topology : topology.name; + return this.delete(`topologies?name=${name}`).toPromise(); + } + + /** + * Creates a new Topology. + * + * @param topology The Topology to create. + */ + public async createTopology(topology: RequestTopology): Promise<ResponseTopology> { + return this.post<ResponseTopology>("topologies", topology).toPromise(); + } + + /** + * Replaces an existing Topology with the provided new definition of a + * Topology. + * + * @param topology The full new definition of the Topology being updated + */ + public async updateTopology(topology: ResponseTopology): Promise<ResponseTopology> { + return this.put<ResponseTopology>(`topologies?name=${topology.name}`, topology).toPromise(); + } + + /** + * Generates a material tree from a topology. + * + * @param topology The topology to generate a material tree from. + * @returns a material tree. + */ + public topologyToTree(topology: ResponseTopology): Array<TopTreeNode> { + const treeNodes: Array<TopTreeNode> = []; + const topLevel: Array<TopTreeNode> = []; + for (const node of topology.nodes) { + const name = node.cachegroup; + const cachegroup = node.cachegroup; + const children: Array<TopTreeNode> = []; + const parents: Array<TopTreeNode> = []; + treeNodes.push({ + cachegroup, + children, + name, + parents, + }); + } + for (let index = 0; index < topology.nodes.length; index++) { + const node = topology.nodes[index]; + const treeNode = treeNodes[index]; + if (!(node.parents instanceof Array) || node.parents.length < 1) { + topLevel.push(treeNode); + continue; + } + for (const parent of node.parents) { + treeNodes[parent].children.push(treeNode); + treeNode.parents.push(treeNodes[parent]); + } + } + return topLevel; + } + + /** + * Generates a topology from a material tree. + * + * @param name The topology name + * @param description The topology description + * @param treeNodes The data for a material tree + * @returns a material tree. + */ + public treeToTopology(name: string, description: string, treeNodes: Array<TopTreeNode>): ResponseTopology { + const topologyNodeIndicesByCacheGroup: Map<string, number> = new Map(); + const nodes: Array<ResponseTopologyNode> = new Array<ResponseTopologyNode>(); + this.treeToTopologyInner(topologyNodeIndicesByCacheGroup, nodes, undefined, treeNodes); + const topology: ResponseTopology = { + description, + lastUpdated: new Date(), + name, + nodes, + }; + return topology; + } + + /** + * Inner recursive function for generating a Topology from a material tree. + * + * @param topologyNodeIndicesByCacheGroup A map of Topology node indices + * using cache group names as the key + * @param topologyNodes The mutable array of Topology nodes + * @param parent The parent, if it exists + * @param treeNodes The data for a material tree + */ + protected treeToTopologyInner(topologyNodeIndicesByCacheGroup: Map<string, number>, + topologyNodes: Array<ResponseTopologyNode>, parent: ResponseTopologyNode | undefined, treeNodes: Array<TopTreeNode>): void { + + for (const treeNode of treeNodes) { + const cachegroup = treeNode.cachegroup; + const parents: number[] = []; + if (parent instanceof Object) { Review Comment: You should pretty much never use `instanceof Object` because that doesn't tell you much of anything: ```javascript [] instanceof Object // true new Number() instanceof Object // true new Date() instanceof Object // true (() => {}) instanceof Object // true ``` If you want to know if something is an `object` primitive type, use `typeof(x) === "object"`, if you want to know if it's an instance of some specific class, you can use `x instanceof Class`, but nearly everything is an `Object`. Note also some weird behavior with null values: ```javascript typeof(null) // "object" null instanceof Object // false ``` ########## experimental/traffic-portal/src/app/core/topologies/topology-details/topology-details.component.ts: ########## @@ -0,0 +1,152 @@ +/* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ +import { NestedTreeControl } from "@angular/cdk/tree"; +import { Location } from "@angular/common"; +import { Component, OnInit } from "@angular/core"; +import { MatDialog } from "@angular/material/dialog"; +import { MatTreeNestedDataSource } from "@angular/material/tree"; +import { ActivatedRoute } from "@angular/router"; +import { ResponseTopology } from "trafficops-types"; + +import { TopologyService, TopTreeNode } from "src/app/api"; +import { + DecisionDialogComponent, + DecisionDialogData, +} from "src/app/shared/dialogs/decision-dialog/decision-dialog.component"; +import { + NavigationService +} from "src/app/shared/navigation/navigation.service"; + +/** + * TopologyDetailComponent is the controller for a Topology's "detail" page. + */ +@Component({ + selector: "tp-topology-details", + styleUrls: ["./topology-details.component.scss"], + templateUrl: "./topology-details.component.html", +}) +export class TopologyDetailsComponent implements OnInit { + public new = false; + public topology: ResponseTopology = { + description: "", + lastUpdated: new Date(), + name: "", + nodes: [], + }; + public showErrors = false; + public topologies: Array<ResponseTopology> = []; + public topologySource = new MatTreeNestedDataSource<TopTreeNode>(); + public topologyControl = new NestedTreeControl<TopTreeNode>(node => node.children); + + constructor( + private readonly route: ActivatedRoute, + private readonly api: TopologyService, + private readonly location: Location, + private readonly dialog: MatDialog, + private readonly navSvc: NavigationService, + ) { + } + + /** + * Angular lifecycle hook where data is initialized. + */ + public async ngOnInit(): Promise<void> { + const name = this.route.snapshot.paramMap.get("name"); + if (name === null) { + console.error("missing required route parameter 'name'"); + return; + } + + const topologiesPromise = this.api.getTopologies().then(topologies => this.topologies = topologies); + if (name === "new") { Review Comment: This means I can't ever create/manipulate a Topology named "new" in this UI - you can look at the server capabilities page source to see how we've worked around this before. ########## experimental/traffic-portal/src/app/core/topologies/topology-details/topology-details.component.ts: ########## @@ -0,0 +1,152 @@ +/* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ +import { NestedTreeControl } from "@angular/cdk/tree"; +import { Location } from "@angular/common"; +import { Component, OnInit } from "@angular/core"; +import { MatDialog } from "@angular/material/dialog"; +import { MatTreeNestedDataSource } from "@angular/material/tree"; +import { ActivatedRoute } from "@angular/router"; +import { ResponseTopology } from "trafficops-types"; + +import { TopologyService, TopTreeNode } from "src/app/api"; +import { + DecisionDialogComponent, + DecisionDialogData, +} from "src/app/shared/dialogs/decision-dialog/decision-dialog.component"; +import { + NavigationService +} from "src/app/shared/navigation/navigation.service"; + +/** + * TopologyDetailComponent is the controller for a Topology's "detail" page. + */ +@Component({ + selector: "tp-topology-details", + styleUrls: ["./topology-details.component.scss"], + templateUrl: "./topology-details.component.html", +}) +export class TopologyDetailsComponent implements OnInit { + public new = false; + public topology: ResponseTopology = { + description: "", + lastUpdated: new Date(), + name: "", + nodes: [], + }; + public showErrors = false; + public topologies: Array<ResponseTopology> = []; + public topologySource = new MatTreeNestedDataSource<TopTreeNode>(); + public topologyControl = new NestedTreeControl<TopTreeNode>(node => node.children); + + constructor( + private readonly route: ActivatedRoute, + private readonly api: TopologyService, + private readonly location: Location, + private readonly dialog: MatDialog, + private readonly navSvc: NavigationService, + ) { + } + + /** + * Angular lifecycle hook where data is initialized. + */ + public async ngOnInit(): Promise<void> { + const name = this.route.snapshot.paramMap.get("name"); + if (name === null) { + console.error("missing required route parameter 'name'"); + return; + } + + const topologiesPromise = this.api.getTopologies().then(topologies => this.topologies = topologies); + if (name === "new") { + this.new = true; + this.setTitle(); + await topologiesPromise; + return; + } + await topologiesPromise; + const index = this.topologies.findIndex(c => c.name === name); + if (index < 0) { + console.error(`no such Topology: #${name}`); Review Comment: Shouldn't use a number sign with a non-numeric identifier ########## experimental/traffic-portal/src/app/core/topologies/topology-details/topology-details.component.ts: ########## @@ -0,0 +1,152 @@ +/* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ +import { NestedTreeControl } from "@angular/cdk/tree"; +import { Location } from "@angular/common"; +import { Component, OnInit } from "@angular/core"; +import { MatDialog } from "@angular/material/dialog"; +import { MatTreeNestedDataSource } from "@angular/material/tree"; +import { ActivatedRoute } from "@angular/router"; +import { ResponseTopology } from "trafficops-types"; + +import { TopologyService, TopTreeNode } from "src/app/api"; +import { + DecisionDialogComponent, + DecisionDialogData, +} from "src/app/shared/dialogs/decision-dialog/decision-dialog.component"; +import { + NavigationService +} from "src/app/shared/navigation/navigation.service"; + +/** + * TopologyDetailComponent is the controller for a Topology's "detail" page. + */ +@Component({ + selector: "tp-topology-details", + styleUrls: ["./topology-details.component.scss"], + templateUrl: "./topology-details.component.html", +}) +export class TopologyDetailsComponent implements OnInit { + public new = false; + public topology: ResponseTopology = { + description: "", + lastUpdated: new Date(), + name: "", + nodes: [], + }; + public showErrors = false; + public topologies: Array<ResponseTopology> = []; + public topologySource = new MatTreeNestedDataSource<TopTreeNode>(); + public topologyControl = new NestedTreeControl<TopTreeNode>(node => node.children); + + constructor( + private readonly route: ActivatedRoute, + private readonly api: TopologyService, + private readonly location: Location, + private readonly dialog: MatDialog, + private readonly navSvc: NavigationService, + ) { + } + + /** + * Angular lifecycle hook where data is initialized. + */ + public async ngOnInit(): Promise<void> { + const name = this.route.snapshot.paramMap.get("name"); + if (name === null) { + console.error("missing required route parameter 'name'"); + return; + } + + const topologiesPromise = this.api.getTopologies().then(topologies => this.topologies = topologies); + if (name === "new") { + this.new = true; + this.setTitle(); + await topologiesPromise; + return; + } + await topologiesPromise; + const index = this.topologies.findIndex(c => c.name === name); + if (index < 0) { + console.error(`no such Topology: #${name}`); + return; + } + this.topology = this.topologies.splice(index, 1)[0]; + this.topologySource.data = this.api.topologyToTree(this.topology); + } + + /** + * Used by angular to determine if this node should be a nested tree node. + * + * @param _ Index of the current node. + * @param node Node to test. + * @returns If the node has children. + */ + public hasChild(_: number, node: TopTreeNode): boolean { + return node.children instanceof Array && node.children.length > 0; Review Comment: Should use `Array.isArray` ########## experimental/traffic-portal/src/app/api/testing/topology.service.ts: ########## @@ -0,0 +1,130 @@ +/* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ + +import { Injectable } from "@angular/core"; +import { + RequestTopology, + ResponseTopology, + ResponseTopologyNode +} from "trafficops-types"; + +/** + * TopologyService expose API functionality relating to Topologies. + */ +@Injectable() +export class TopologyService { + private readonly topologies: ResponseTopology[] = [ + { + description: "", + lastUpdated: new Date(), + name: "test", + nodes: [ + { + cachegroup: "Edge", + parents: [1], + }, + { + cachegroup: "Mid", + parents: [2], + }, + { + cachegroup: "Origin", + parents: [], + }, + ], + } + ]; + + /** + * Gets one or all Topologies from Traffic Ops + * + * @param name? The integral, unique identifier of a single Topology to be Review Comment: Identifiers for Topologies are not integral. ########## experimental/traffic-portal/src/app/api/testing/topology.service.ts: ########## @@ -0,0 +1,130 @@ +/* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ + +import { Injectable } from "@angular/core"; +import { + RequestTopology, + ResponseTopology, + ResponseTopologyNode +} from "trafficops-types"; + +/** + * TopologyService expose API functionality relating to Topologies. + */ +@Injectable() +export class TopologyService { Review Comment: Testing service is missing the non-API methods of the concrete service ########## experimental/traffic-portal/src/app/api/testing/topology.service.ts: ########## @@ -0,0 +1,130 @@ +/* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ + +import { Injectable } from "@angular/core"; +import { + RequestTopology, + ResponseTopology, + ResponseTopologyNode +} from "trafficops-types"; + +/** + * TopologyService expose API functionality relating to Topologies. + */ +@Injectable() +export class TopologyService { + private readonly topologies: ResponseTopology[] = [ + { + description: "", + lastUpdated: new Date(), + name: "test", + nodes: [ + { + cachegroup: "Edge", + parents: [1], + }, + { + cachegroup: "Mid", + parents: [2], + }, + { + cachegroup: "Origin", + parents: [], + }, + ], + } + ]; + + /** + * Gets one or all Topologies from Traffic Ops + * + * @param name? The integral, unique identifier of a single Topology to be + * returned + * @returns Either a Map of Topology names to full Topology objects, or a single Topology, depending on whether `id` was + * passed. + * (In the event that `id` is passed but does not match any Topology, `null` will be emitted) + */ + public async getTopologies(name?: string): Promise<Array<ResponseTopology> | ResponseTopology> { + if (name !== undefined) { + const topology = this.topologies.find(t => t.name === name); + if (!topology) { + throw new Error(`no such Topology #${name}`); + } + return topology; + } + return this.topologies; + } + + /** + * Deletes a Topology. + * + * @param topology The Topology to be deleted, or just its ID. + */ + public async deleteTopology(topology: ResponseTopology | string): Promise<void> { + const name = typeof topology === "string" ? topology : topology.name; + const idx = this.topologies.findIndex(t => t.name === name); + if (idx < 0) { + throw new Error(`no such Topology: #${name}`); + } + this.topologies.splice(idx, 1); + } + + /** + * Creates a new Topology. + * + * @param topology The Topology to create. + */ + public async createTopology(topology: RequestTopology): Promise<ResponseTopology> { + const nodes: ResponseTopologyNode[] = topology.nodes.map(node => { + if (!(node.parents instanceof Array)) { + node.parents = []; + } + const responseNode: ResponseTopologyNode = { + cachegroup: node.cachegroup, + parents: node.parents || [], + }; + return responseNode; + }); + const t: ResponseTopology = { + description: topology.description || "", + lastUpdated: new Date(), + name: topology.name, + nodes, + }; + this.topologies.push(t); + return t; + } + + /** + * Replaces an existing Topology with the provided new definition of a + * Topology. + * + * @param topology The full new definition of the Topology being + * updated, or just its ID. + */ + public async updateTopology(topology: ResponseTopology): Promise<ResponseTopology> { + const idx = this.topologies.findIndex(t => t.name === topology.name); + topology = { + ...topology, + lastUpdated: new Date() + }; + + if (idx < 0) { + throw new Error(`no such Topology: #${topology}`); Review Comment: Shouldn't put a number sign before a non-numeric identifier ########## experimental/traffic-portal/src/app/api/topology.service.ts: ########## @@ -0,0 +1,196 @@ +/* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ +import { HttpClient } from "@angular/common/http"; +import { Injectable } from "@angular/core"; +import type { + RequestTopology, + ResponseTopology, + ResponseTopologyNode, +} from "trafficops-types"; + +import { APIService } from "./base-api.service"; + +/** + * TopTreeNode is used to represent a topology in a format usable as a material + * nested tree data source. + */ +export interface TopTreeNode { + name: string; + cachegroup: string; + children: Array<TopTreeNode>; + parents: Array<this>; +} + +/** + * TopologyService exposes API functionality relating to Topologies. + */ +@Injectable() +export class TopologyService extends APIService { + + constructor(http: HttpClient) { + super(http); + } + + /** + * Gets a specific Topology from Traffic Ops + * + * @param name The name of the Topology to be returned. + * @returns The Topology with the given name. + */ + public async getTopologies(name: string): Promise<ResponseTopology>; + /** + * Gets all Topologies from Traffic Ops + * + * @returns An Array of all Topologies configured in Traffic Ops. + */ + public async getTopologies(): Promise<Array<ResponseTopology>>; + /** + * Gets one or all Topologies from Traffic Ops + * + * @param name The name of a single Topology to be returned. + * @returns Either an Array of Topology objects, or a single Topology, depending on + * whether `name` was passed. + */ + public async getTopologies(name?: string): Promise<Array<ResponseTopology> | ResponseTopology> { + const path = "topologies"; + if (name) { + const topology = await this.get<[ResponseTopology]>(path, undefined, {name}).toPromise(); + if (topology.length !== 1) { + throw new Error(`${topology.length} Topologies found by name ${name}`); + } + return topology[0]; + } + return this.get<Array<ResponseTopology>>(path).toPromise(); + } + + /** + * Deletes a Topology. + * + * @param topology The Topology to be deleted, or just its name. + */ + public async deleteTopology(topology: ResponseTopology | string): Promise<void> { + const name = typeof topology === "string" ? topology : topology.name; + return this.delete(`topologies?name=${name}`).toPromise(); Review Comment: passing in query string parameters should be done using the appropriate argument to the API method, not string interpolation. ########## experimental/traffic-portal/src/app/api/testing/topology.service.ts: ########## @@ -0,0 +1,130 @@ +/* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ + +import { Injectable } from "@angular/core"; +import { + RequestTopology, + ResponseTopology, + ResponseTopologyNode +} from "trafficops-types"; + +/** + * TopologyService expose API functionality relating to Topologies. + */ +@Injectable() +export class TopologyService { + private readonly topologies: ResponseTopology[] = [ + { + description: "", + lastUpdated: new Date(), + name: "test", + nodes: [ + { + cachegroup: "Edge", + parents: [1], + }, + { + cachegroup: "Mid", + parents: [2], + }, + { + cachegroup: "Origin", + parents: [], + }, + ], + } + ]; + + /** + * Gets one or all Topologies from Traffic Ops + * + * @param name? The integral, unique identifier of a single Topology to be + * returned + * @returns Either a Map of Topology names to full Topology objects, or a single Topology, depending on whether `id` was + * passed. + * (In the event that `id` is passed but does not match any Topology, `null` will be emitted) + */ + public async getTopologies(name?: string): Promise<Array<ResponseTopology> | ResponseTopology> { + if (name !== undefined) { + const topology = this.topologies.find(t => t.name === name); + if (!topology) { + throw new Error(`no such Topology #${name}`); + } + return topology; + } + return this.topologies; + } + + /** + * Deletes a Topology. + * + * @param topology The Topology to be deleted, or just its ID. + */ + public async deleteTopology(topology: ResponseTopology | string): Promise<void> { + const name = typeof topology === "string" ? topology : topology.name; + const idx = this.topologies.findIndex(t => t.name === name); + if (idx < 0) { + throw new Error(`no such Topology: #${name}`); + } + this.topologies.splice(idx, 1); + } + + /** + * Creates a new Topology. + * + * @param topology The Topology to create. + */ + public async createTopology(topology: RequestTopology): Promise<ResponseTopology> { + const nodes: ResponseTopologyNode[] = topology.nodes.map(node => { + if (!(node.parents instanceof Array)) { + node.parents = []; + } + const responseNode: ResponseTopologyNode = { + cachegroup: node.cachegroup, + parents: node.parents || [], + }; + return responseNode; + }); + const t: ResponseTopology = { + description: topology.description || "", + lastUpdated: new Date(), + name: topology.name, + nodes, + }; + this.topologies.push(t); + return t; + } + + /** + * Replaces an existing Topology with the provided new definition of a + * Topology. + * + * @param topology The full new definition of the Topology being + * updated, or just its ID. Review Comment: Topologies don't have IDs, and this method doesn't accept more than one type of argument. ########## experimental/traffic-portal/src/app/api/topology.service.ts: ########## @@ -0,0 +1,196 @@ +/* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ +import { HttpClient } from "@angular/common/http"; +import { Injectable } from "@angular/core"; +import type { + RequestTopology, + ResponseTopology, + ResponseTopologyNode, +} from "trafficops-types"; + +import { APIService } from "./base-api.service"; + +/** + * TopTreeNode is used to represent a topology in a format usable as a material + * nested tree data source. + */ +export interface TopTreeNode { + name: string; + cachegroup: string; + children: Array<TopTreeNode>; + parents: Array<this>; +} + +/** + * TopologyService exposes API functionality relating to Topologies. + */ +@Injectable() +export class TopologyService extends APIService { + + constructor(http: HttpClient) { + super(http); + } + + /** + * Gets a specific Topology from Traffic Ops + * + * @param name The name of the Topology to be returned. + * @returns The Topology with the given name. + */ + public async getTopologies(name: string): Promise<ResponseTopology>; + /** + * Gets all Topologies from Traffic Ops + * + * @returns An Array of all Topologies configured in Traffic Ops. + */ + public async getTopologies(): Promise<Array<ResponseTopology>>; + /** + * Gets one or all Topologies from Traffic Ops + * + * @param name The name of a single Topology to be returned. + * @returns Either an Array of Topology objects, or a single Topology, depending on + * whether `name` was passed. + */ + public async getTopologies(name?: string): Promise<Array<ResponseTopology> | ResponseTopology> { + const path = "topologies"; + if (name) { + const topology = await this.get<[ResponseTopology]>(path, undefined, {name}).toPromise(); + if (topology.length !== 1) { + throw new Error(`${topology.length} Topologies found by name ${name}`); + } + return topology[0]; + } + return this.get<Array<ResponseTopology>>(path).toPromise(); + } + + /** + * Deletes a Topology. + * + * @param topology The Topology to be deleted, or just its name. + */ + public async deleteTopology(topology: ResponseTopology | string): Promise<void> { + const name = typeof topology === "string" ? topology : topology.name; + return this.delete(`topologies?name=${name}`).toPromise(); + } + + /** + * Creates a new Topology. + * + * @param topology The Topology to create. + */ + public async createTopology(topology: RequestTopology): Promise<ResponseTopology> { + return this.post<ResponseTopology>("topologies", topology).toPromise(); + } + + /** + * Replaces an existing Topology with the provided new definition of a + * Topology. + * + * @param topology The full new definition of the Topology being updated + */ + public async updateTopology(topology: ResponseTopology): Promise<ResponseTopology> { + return this.put<ResponseTopology>(`topologies?name=${topology.name}`, topology).toPromise(); + } + + /** + * Generates a material tree from a topology. + * + * @param topology The topology to generate a material tree from. + * @returns a material tree. + */ + public topologyToTree(topology: ResponseTopology): Array<TopTreeNode> { Review Comment: Methods that don't need access to instance methods or data members should be static - or just functions, although I think a static method makes sense for this. ########## experimental/traffic-portal/src/app/api/topology.service.ts: ########## @@ -0,0 +1,196 @@ +/* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ +import { HttpClient } from "@angular/common/http"; +import { Injectable } from "@angular/core"; +import type { + RequestTopology, + ResponseTopology, + ResponseTopologyNode, +} from "trafficops-types"; + +import { APIService } from "./base-api.service"; + +/** + * TopTreeNode is used to represent a topology in a format usable as a material + * nested tree data source. + */ +export interface TopTreeNode { + name: string; + cachegroup: string; + children: Array<TopTreeNode>; + parents: Array<this>; +} + +/** + * TopologyService exposes API functionality relating to Topologies. + */ +@Injectable() +export class TopologyService extends APIService { + + constructor(http: HttpClient) { + super(http); + } + + /** + * Gets a specific Topology from Traffic Ops + * + * @param name The name of the Topology to be returned. + * @returns The Topology with the given name. + */ + public async getTopologies(name: string): Promise<ResponseTopology>; + /** + * Gets all Topologies from Traffic Ops + * + * @returns An Array of all Topologies configured in Traffic Ops. + */ + public async getTopologies(): Promise<Array<ResponseTopology>>; + /** + * Gets one or all Topologies from Traffic Ops + * + * @param name The name of a single Topology to be returned. + * @returns Either an Array of Topology objects, or a single Topology, depending on + * whether `name` was passed. + */ + public async getTopologies(name?: string): Promise<Array<ResponseTopology> | ResponseTopology> { + const path = "topologies"; + if (name) { + const topology = await this.get<[ResponseTopology]>(path, undefined, {name}).toPromise(); + if (topology.length !== 1) { + throw new Error(`${topology.length} Topologies found by name ${name}`); + } + return topology[0]; + } + return this.get<Array<ResponseTopology>>(path).toPromise(); + } + + /** + * Deletes a Topology. + * + * @param topology The Topology to be deleted, or just its name. + */ + public async deleteTopology(topology: ResponseTopology | string): Promise<void> { + const name = typeof topology === "string" ? topology : topology.name; + return this.delete(`topologies?name=${name}`).toPromise(); + } + + /** + * Creates a new Topology. + * + * @param topology The Topology to create. + */ + public async createTopology(topology: RequestTopology): Promise<ResponseTopology> { + return this.post<ResponseTopology>("topologies", topology).toPromise(); + } + + /** + * Replaces an existing Topology with the provided new definition of a + * Topology. + * + * @param topology The full new definition of the Topology being updated + */ + public async updateTopology(topology: ResponseTopology): Promise<ResponseTopology> { + return this.put<ResponseTopology>(`topologies?name=${topology.name}`, topology).toPromise(); + } + + /** + * Generates a material tree from a topology. + * + * @param topology The topology to generate a material tree from. + * @returns a material tree. + */ + public topologyToTree(topology: ResponseTopology): Array<TopTreeNode> { + const treeNodes: Array<TopTreeNode> = []; + const topLevel: Array<TopTreeNode> = []; + for (const node of topology.nodes) { + const name = node.cachegroup; + const cachegroup = node.cachegroup; + const children: Array<TopTreeNode> = []; + const parents: Array<TopTreeNode> = []; + treeNodes.push({ + cachegroup, + children, + name, + parents, + }); + } + for (let index = 0; index < topology.nodes.length; index++) { + const node = topology.nodes[index]; + const treeNode = treeNodes[index]; + if (!(node.parents instanceof Array) || node.parents.length < 1) { Review Comment: checking for arrays should use `Array.isArray`. The reason for that is because `instanceof` checks to see if the first operand is an instantiation of the second operand or some descendant thereof. The problem with that is that one "realm"'s `Array` doesn't compare equal with any other realm's `Array`, so when used across realms `instanceof Array` will report `false` even when used on an Array. That's a situation we don't anticipate, but because there's no other practical difference between the approaches, we should prefer the one that will always work. ########## experimental/traffic-portal/src/app/api/topology.service.ts: ########## @@ -0,0 +1,196 @@ +/* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ +import { HttpClient } from "@angular/common/http"; +import { Injectable } from "@angular/core"; +import type { + RequestTopology, + ResponseTopology, + ResponseTopologyNode, +} from "trafficops-types"; + +import { APIService } from "./base-api.service"; + +/** + * TopTreeNode is used to represent a topology in a format usable as a material + * nested tree data source. + */ +export interface TopTreeNode { + name: string; + cachegroup: string; + children: Array<TopTreeNode>; + parents: Array<this>; +} + +/** + * TopologyService exposes API functionality relating to Topologies. + */ +@Injectable() +export class TopologyService extends APIService { + + constructor(http: HttpClient) { + super(http); + } + + /** + * Gets a specific Topology from Traffic Ops + * + * @param name The name of the Topology to be returned. + * @returns The Topology with the given name. + */ + public async getTopologies(name: string): Promise<ResponseTopology>; + /** + * Gets all Topologies from Traffic Ops + * + * @returns An Array of all Topologies configured in Traffic Ops. + */ + public async getTopologies(): Promise<Array<ResponseTopology>>; + /** + * Gets one or all Topologies from Traffic Ops + * + * @param name The name of a single Topology to be returned. + * @returns Either an Array of Topology objects, or a single Topology, depending on + * whether `name` was passed. + */ + public async getTopologies(name?: string): Promise<Array<ResponseTopology> | ResponseTopology> { + const path = "topologies"; + if (name) { + const topology = await this.get<[ResponseTopology]>(path, undefined, {name}).toPromise(); + if (topology.length !== 1) { + throw new Error(`${topology.length} Topologies found by name ${name}`); + } + return topology[0]; + } + return this.get<Array<ResponseTopology>>(path).toPromise(); + } + + /** + * Deletes a Topology. + * + * @param topology The Topology to be deleted, or just its name. + */ + public async deleteTopology(topology: ResponseTopology | string): Promise<void> { + const name = typeof topology === "string" ? topology : topology.name; + return this.delete(`topologies?name=${name}`).toPromise(); + } + + /** + * Creates a new Topology. + * + * @param topology The Topology to create. + */ + public async createTopology(topology: RequestTopology): Promise<ResponseTopology> { + return this.post<ResponseTopology>("topologies", topology).toPromise(); + } + + /** + * Replaces an existing Topology with the provided new definition of a + * Topology. + * + * @param topology The full new definition of the Topology being updated + */ + public async updateTopology(topology: ResponseTopology): Promise<ResponseTopology> { + return this.put<ResponseTopology>(`topologies?name=${topology.name}`, topology).toPromise(); + } + + /** + * Generates a material tree from a topology. + * + * @param topology The topology to generate a material tree from. + * @returns a material tree. + */ + public topologyToTree(topology: ResponseTopology): Array<TopTreeNode> { + const treeNodes: Array<TopTreeNode> = []; + const topLevel: Array<TopTreeNode> = []; + for (const node of topology.nodes) { + const name = node.cachegroup; + const cachegroup = node.cachegroup; + const children: Array<TopTreeNode> = []; + const parents: Array<TopTreeNode> = []; + treeNodes.push({ + cachegroup, + children, + name, + parents, + }); + } + for (let index = 0; index < topology.nodes.length; index++) { + const node = topology.nodes[index]; + const treeNode = treeNodes[index]; + if (!(node.parents instanceof Array) || node.parents.length < 1) { + topLevel.push(treeNode); + continue; + } + for (const parent of node.parents) { + treeNodes[parent].children.push(treeNode); + treeNode.parents.push(treeNodes[parent]); + } + } + return topLevel; + } + + /** + * Generates a topology from a material tree. + * + * @param name The topology name + * @param description The topology description + * @param treeNodes The data for a material tree + * @returns a material tree. + */ + public treeToTopology(name: string, description: string, treeNodes: Array<TopTreeNode>): ResponseTopology { + const topologyNodeIndicesByCacheGroup: Map<string, number> = new Map(); + const nodes: Array<ResponseTopologyNode> = new Array<ResponseTopologyNode>(); + this.treeToTopologyInner(topologyNodeIndicesByCacheGroup, nodes, undefined, treeNodes); + const topology: ResponseTopology = { + description, + lastUpdated: new Date(), + name, + nodes, + }; + return topology; + } + + /** + * Inner recursive function for generating a Topology from a material tree. + * + * @param topologyNodeIndicesByCacheGroup A map of Topology node indices + * using cache group names as the key + * @param topologyNodes The mutable array of Topology nodes + * @param parent The parent, if it exists + * @param treeNodes The data for a material tree + */ + protected treeToTopologyInner(topologyNodeIndicesByCacheGroup: Map<string, number>, + topologyNodes: Array<ResponseTopologyNode>, parent: ResponseTopologyNode | undefined, treeNodes: Array<TopTreeNode>): void { Review Comment: you have ``` publicity methodName(arg0, arg1, arg2): returnType { ``` which is inconsistent and difficult to read. You _can_ do ``` publicity methodName(arg0, arg1, arg2): returnType { ``` but IMO that's ugly and I prefer ``` publicity methodName( arg0, arg1, arg2 ): returnType { ``` Either one is fine with the linter and keeps it to one argument per line, so do whatever you think is best. ########## experimental/traffic-portal/src/app/api/topology.service.ts: ########## @@ -0,0 +1,196 @@ +/* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ +import { HttpClient } from "@angular/common/http"; +import { Injectable } from "@angular/core"; +import type { + RequestTopology, + ResponseTopology, + ResponseTopologyNode, +} from "trafficops-types"; + +import { APIService } from "./base-api.service"; + +/** + * TopTreeNode is used to represent a topology in a format usable as a material + * nested tree data source. + */ +export interface TopTreeNode { + name: string; + cachegroup: string; + children: Array<TopTreeNode>; + parents: Array<this>; +} + +/** + * TopologyService exposes API functionality relating to Topologies. + */ +@Injectable() +export class TopologyService extends APIService { + + constructor(http: HttpClient) { + super(http); + } + + /** + * Gets a specific Topology from Traffic Ops + * + * @param name The name of the Topology to be returned. + * @returns The Topology with the given name. + */ + public async getTopologies(name: string): Promise<ResponseTopology>; + /** + * Gets all Topologies from Traffic Ops + * + * @returns An Array of all Topologies configured in Traffic Ops. + */ + public async getTopologies(): Promise<Array<ResponseTopology>>; + /** + * Gets one or all Topologies from Traffic Ops + * + * @param name The name of a single Topology to be returned. + * @returns Either an Array of Topology objects, or a single Topology, depending on + * whether `name` was passed. + */ + public async getTopologies(name?: string): Promise<Array<ResponseTopology> | ResponseTopology> { + const path = "topologies"; + if (name) { + const topology = await this.get<[ResponseTopology]>(path, undefined, {name}).toPromise(); + if (topology.length !== 1) { + throw new Error(`${topology.length} Topologies found by name ${name}`); + } + return topology[0]; + } + return this.get<Array<ResponseTopology>>(path).toPromise(); + } + + /** + * Deletes a Topology. + * + * @param topology The Topology to be deleted, or just its name. + */ + public async deleteTopology(topology: ResponseTopology | string): Promise<void> { + const name = typeof topology === "string" ? topology : topology.name; + return this.delete(`topologies?name=${name}`).toPromise(); + } + + /** + * Creates a new Topology. + * + * @param topology The Topology to create. + */ + public async createTopology(topology: RequestTopology): Promise<ResponseTopology> { + return this.post<ResponseTopology>("topologies", topology).toPromise(); + } + + /** + * Replaces an existing Topology with the provided new definition of a + * Topology. + * + * @param topology The full new definition of the Topology being updated + */ + public async updateTopology(topology: ResponseTopology): Promise<ResponseTopology> { + return this.put<ResponseTopology>(`topologies?name=${topology.name}`, topology).toPromise(); Review Comment: Same as above ########## experimental/traffic-portal/src/app/api/topology.service.ts: ########## @@ -0,0 +1,196 @@ +/* Review Comment: This API service has 0% test coverage ########## experimental/traffic-portal/src/app/api/topology.service.ts: ########## @@ -0,0 +1,196 @@ +/* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ +import { HttpClient } from "@angular/common/http"; +import { Injectable } from "@angular/core"; +import type { + RequestTopology, + ResponseTopology, + ResponseTopologyNode, +} from "trafficops-types"; + +import { APIService } from "./base-api.service"; + +/** + * TopTreeNode is used to represent a topology in a format usable as a material + * nested tree data source. + */ +export interface TopTreeNode { + name: string; + cachegroup: string; + children: Array<TopTreeNode>; + parents: Array<this>; +} + +/** + * TopologyService exposes API functionality relating to Topologies. + */ +@Injectable() +export class TopologyService extends APIService { + + constructor(http: HttpClient) { + super(http); + } + + /** + * Gets a specific Topology from Traffic Ops + * + * @param name The name of the Topology to be returned. + * @returns The Topology with the given name. + */ + public async getTopologies(name: string): Promise<ResponseTopology>; + /** + * Gets all Topologies from Traffic Ops + * + * @returns An Array of all Topologies configured in Traffic Ops. + */ + public async getTopologies(): Promise<Array<ResponseTopology>>; + /** + * Gets one or all Topologies from Traffic Ops + * + * @param name The name of a single Topology to be returned. + * @returns Either an Array of Topology objects, or a single Topology, depending on + * whether `name` was passed. + */ + public async getTopologies(name?: string): Promise<Array<ResponseTopology> | ResponseTopology> { + const path = "topologies"; + if (name) { + const topology = await this.get<[ResponseTopology]>(path, undefined, {name}).toPromise(); + if (topology.length !== 1) { + throw new Error(`${topology.length} Topologies found by name ${name}`); + } + return topology[0]; + } + return this.get<Array<ResponseTopology>>(path).toPromise(); + } + + /** + * Deletes a Topology. + * + * @param topology The Topology to be deleted, or just its name. + */ + public async deleteTopology(topology: ResponseTopology | string): Promise<void> { + const name = typeof topology === "string" ? topology : topology.name; + return this.delete(`topologies?name=${name}`).toPromise(); + } + + /** + * Creates a new Topology. + * + * @param topology The Topology to create. + */ + public async createTopology(topology: RequestTopology): Promise<ResponseTopology> { + return this.post<ResponseTopology>("topologies", topology).toPromise(); + } + + /** + * Replaces an existing Topology with the provided new definition of a + * Topology. + * + * @param topology The full new definition of the Topology being updated + */ + public async updateTopology(topology: ResponseTopology): Promise<ResponseTopology> { + return this.put<ResponseTopology>(`topologies?name=${topology.name}`, topology).toPromise(); + } + + /** + * Generates a material tree from a topology. + * + * @param topology The topology to generate a material tree from. + * @returns a material tree. + */ + public topologyToTree(topology: ResponseTopology): Array<TopTreeNode> { + const treeNodes: Array<TopTreeNode> = []; + const topLevel: Array<TopTreeNode> = []; + for (const node of topology.nodes) { + const name = node.cachegroup; + const cachegroup = node.cachegroup; + const children: Array<TopTreeNode> = []; + const parents: Array<TopTreeNode> = []; + treeNodes.push({ + cachegroup, + children, + name, + parents, + }); + } + for (let index = 0; index < topology.nodes.length; index++) { + const node = topology.nodes[index]; + const treeNode = treeNodes[index]; + if (!(node.parents instanceof Array) || node.parents.length < 1) { + topLevel.push(treeNode); + continue; + } + for (const parent of node.parents) { + treeNodes[parent].children.push(treeNode); + treeNode.parents.push(treeNodes[parent]); + } + } + return topLevel; + } + + /** + * Generates a topology from a material tree. + * + * @param name The topology name + * @param description The topology description + * @param treeNodes The data for a material tree + * @returns a material tree. + */ + public treeToTopology(name: string, description: string, treeNodes: Array<TopTreeNode>): ResponseTopology { Review Comment: methods that don't need `this` should be static ( and since I'd say the same thing about `treeToTopologyInner` it doesn't need `this` to access that, either). ########## experimental/traffic-portal/src/app/core/topologies/topology-details/topology-details.component.spec.ts: ########## @@ -0,0 +1,55 @@ +/* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { MatDialogModule } from "@angular/material/dialog"; +import { ActivatedRoute } from "@angular/router"; +import { RouterTestingModule } from "@angular/router/testing"; +import { ReplaySubject } from "rxjs"; + +import { APITestingModule } from "src/app/api/testing"; +import { + NavigationService +} from "src/app/shared/navigation/navigation.service"; + +import { TopologyDetailsComponent } from "./topology-details.component"; + +describe("TopologyDetailsComponent", () => { + let component: TopologyDetailsComponent; + let fixture: ComponentFixture<TopologyDetailsComponent>; + let route: ActivatedRoute; + let paramMap: jasmine.Spy; Review Comment: `route` and `paramMap` are never used in their declared scope (which could easily change when a non-trivial test is added). ########## experimental/traffic-portal/src/app/core/topologies/topology-details/topology-details.component.spec.ts: ########## @@ -0,0 +1,55 @@ +/* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { MatDialogModule } from "@angular/material/dialog"; +import { ActivatedRoute } from "@angular/router"; +import { RouterTestingModule } from "@angular/router/testing"; +import { ReplaySubject } from "rxjs"; + +import { APITestingModule } from "src/app/api/testing"; +import { + NavigationService +} from "src/app/shared/navigation/navigation.service"; + +import { TopologyDetailsComponent } from "./topology-details.component"; + +describe("TopologyDetailsComponent", () => { + let component: TopologyDetailsComponent; + let fixture: ComponentFixture<TopologyDetailsComponent>; + let route: ActivatedRoute; + let paramMap: jasmine.Spy; + + const navSvc = jasmine.createSpyObj([], { + headerHidden: new ReplaySubject<boolean>(), + headerTitle: new ReplaySubject<string>(), + }); + + beforeEach(() => { + TestBed.configureTestingModule({ + declarations: [TopologyDetailsComponent], + imports: [APITestingModule, RouterTestingModule, MatDialogModule], + providers: [{provide: NavigationService, useValue: navSvc}], + }); + route = TestBed.inject(ActivatedRoute); + paramMap = spyOn(route.snapshot.paramMap, "get"); + paramMap.and.returnValue(null); + fixture = TestBed.createComponent(TopologyDetailsComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + }); + + it("should create", () => { + expect(component).toBeTruthy(); + }); Review Comment: Tests should include at least one non-trivial test ########## experimental/traffic-portal/src/app/api/topology.service.ts: ########## @@ -0,0 +1,196 @@ +/* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ +import { HttpClient } from "@angular/common/http"; +import { Injectable } from "@angular/core"; +import type { + RequestTopology, + ResponseTopology, + ResponseTopologyNode, +} from "trafficops-types"; + +import { APIService } from "./base-api.service"; + +/** + * TopTreeNode is used to represent a topology in a format usable as a material + * nested tree data source. + */ +export interface TopTreeNode { + name: string; + cachegroup: string; + children: Array<TopTreeNode>; + parents: Array<this>; +} + +/** + * TopologyService exposes API functionality relating to Topologies. + */ +@Injectable() +export class TopologyService extends APIService { + + constructor(http: HttpClient) { + super(http); + } + + /** + * Gets a specific Topology from Traffic Ops + * + * @param name The name of the Topology to be returned. + * @returns The Topology with the given name. + */ + public async getTopologies(name: string): Promise<ResponseTopology>; + /** + * Gets all Topologies from Traffic Ops + * + * @returns An Array of all Topologies configured in Traffic Ops. + */ + public async getTopologies(): Promise<Array<ResponseTopology>>; + /** + * Gets one or all Topologies from Traffic Ops + * + * @param name The name of a single Topology to be returned. + * @returns Either an Array of Topology objects, or a single Topology, depending on + * whether `name` was passed. + */ + public async getTopologies(name?: string): Promise<Array<ResponseTopology> | ResponseTopology> { + const path = "topologies"; + if (name) { + const topology = await this.get<[ResponseTopology]>(path, undefined, {name}).toPromise(); + if (topology.length !== 1) { + throw new Error(`${topology.length} Topologies found by name ${name}`); + } + return topology[0]; + } + return this.get<Array<ResponseTopology>>(path).toPromise(); + } + + /** + * Deletes a Topology. + * + * @param topology The Topology to be deleted, or just its name. + */ + public async deleteTopology(topology: ResponseTopology | string): Promise<void> { + const name = typeof topology === "string" ? topology : topology.name; + return this.delete(`topologies?name=${name}`).toPromise(); + } + + /** + * Creates a new Topology. + * + * @param topology The Topology to create. + */ + public async createTopology(topology: RequestTopology): Promise<ResponseTopology> { + return this.post<ResponseTopology>("topologies", topology).toPromise(); + } + + /** + * Replaces an existing Topology with the provided new definition of a + * Topology. + * + * @param topology The full new definition of the Topology being updated + */ + public async updateTopology(topology: ResponseTopology): Promise<ResponseTopology> { + return this.put<ResponseTopology>(`topologies?name=${topology.name}`, topology).toPromise(); + } + + /** + * Generates a material tree from a topology. + * + * @param topology The topology to generate a material tree from. + * @returns a material tree. + */ + public topologyToTree(topology: ResponseTopology): Array<TopTreeNode> { + const treeNodes: Array<TopTreeNode> = []; + const topLevel: Array<TopTreeNode> = []; + for (const node of topology.nodes) { + const name = node.cachegroup; + const cachegroup = node.cachegroup; + const children: Array<TopTreeNode> = []; + const parents: Array<TopTreeNode> = []; + treeNodes.push({ + cachegroup, + children, + name, + parents, + }); + } + for (let index = 0; index < topology.nodes.length; index++) { + const node = topology.nodes[index]; + const treeNode = treeNodes[index]; + if (!(node.parents instanceof Array) || node.parents.length < 1) { + topLevel.push(treeNode); + continue; + } + for (const parent of node.parents) { + treeNodes[parent].children.push(treeNode); + treeNode.parents.push(treeNodes[parent]); + } + } + return topLevel; + } + + /** + * Generates a topology from a material tree. + * + * @param name The topology name + * @param description The topology description + * @param treeNodes The data for a material tree + * @returns a material tree. + */ + public treeToTopology(name: string, description: string, treeNodes: Array<TopTreeNode>): ResponseTopology { + const topologyNodeIndicesByCacheGroup: Map<string, number> = new Map(); + const nodes: Array<ResponseTopologyNode> = new Array<ResponseTopologyNode>(); + this.treeToTopologyInner(topologyNodeIndicesByCacheGroup, nodes, undefined, treeNodes); + const topology: ResponseTopology = { + description, + lastUpdated: new Date(), + name, + nodes, + }; + return topology; + } + + /** + * Inner recursive function for generating a Topology from a material tree. + * + * @param topologyNodeIndicesByCacheGroup A map of Topology node indices + * using cache group names as the key + * @param topologyNodes The mutable array of Topology nodes + * @param parent The parent, if it exists + * @param treeNodes The data for a material tree + */ + protected treeToTopologyInner(topologyNodeIndicesByCacheGroup: Map<string, number>, Review Comment: Same as above RE: "material tree" & static methods ########## experimental/traffic-portal/src/app/core/topologies/topology-details/topology-details.component.html: ########## @@ -0,0 +1,45 @@ +<!-- +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +--> +<mat-card appearance="outlined" class="page-content single-column"> + <tp-loading *ngIf="!topology"></tp-loading> + <form ngNativeValidate (ngSubmit)="submit($event)" *ngIf="topology"> + <mat-card-content class="container"> + <mat-form-field> + <mat-label>Name</mat-label> + <input matInput type="text" name="name" required readonly disabled [(ngModel)]="topology.name"> + </mat-form-field> + <mat-form-field *ngIf="!new"> + <mat-label>Description</mat-label> + <textarea matInput name="description" [(ngModel)]="topology.description"></textarea> + </mat-form-field> + </mat-card-content> + <mat-card-content class="container"> + <mat-tree [dataSource]="topologySource" [treeControl]="topologyControl"> + <mat-nested-tree-node *matTreeNodeDef="let node; when: hasChild"> + <div class="mat-tree-node" matTreeNodeToggle mat-menu-item [attr.aria-label]="'Toggle ' + node.name"> + {{node.name}} + </div> + <div class="expand-node" role="group"> + <ng-container matTreeNodeOutlet></ng-container> + </div> + <!--<mat-divider *ngIf="(node)"></mat-divider>--> Review Comment: should remove unused line ########## experimental/traffic-portal/src/app/core/topologies/topology-details/topology-details.component.html: ########## @@ -0,0 +1,45 @@ +<!-- +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +--> +<mat-card appearance="outlined" class="page-content single-column"> + <tp-loading *ngIf="!topology"></tp-loading> + <form ngNativeValidate (ngSubmit)="submit($event)" *ngIf="topology"> Review Comment: This `ngIf` directive will never evaluate to `false` ########## experimental/traffic-portal/src/app/core/topologies/topology-details/topology-details.component.html: ########## @@ -0,0 +1,45 @@ +<!-- +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +--> +<mat-card appearance="outlined" class="page-content single-column"> + <tp-loading *ngIf="!topology"></tp-loading> + <form ngNativeValidate (ngSubmit)="submit($event)" *ngIf="topology"> + <mat-card-content class="container"> + <mat-form-field> + <mat-label>Name</mat-label> + <input matInput type="text" name="name" required readonly disabled [(ngModel)]="topology.name"> Review Comment: a two-way binding is useless on a readonly control; should bind to `[value]` instead of `[(ngModel)]` ########## experimental/traffic-portal/src/app/core/topologies/topology-details/topology-details.component.html: ########## @@ -0,0 +1,45 @@ +<!-- +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +--> +<mat-card appearance="outlined" class="page-content single-column"> + <tp-loading *ngIf="!topology"></tp-loading> Review Comment: This `ngIf` directive will never evaluate to `true` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
