ocket8888 commented on code in PR #7000: URL: https://github.com/apache/trafficcontrol/pull/7000#discussion_r936053642
########## experimental/traffic-portal/src/app/core/users/tenants/tenant-details/tenant-details.component.ts: ########## @@ -0,0 +1,175 @@ +/* +* 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 { Location } from "@angular/common"; +import { Component, OnInit } from "@angular/core"; +import { ActivatedRoute } from "@angular/router"; +import { RequestTenant, ResponseTenant, Tenant } from "trafficops-types"; + +import { UserService } from "src/app/api"; +import { TreeData } from "src/app/models/tree-select.model"; + +/** + * TenantsDetailsComponent is the controller for thee tenant add/edit form. + */ +@Component({ + selector: "tp-tenant-details", + styleUrls: ["./tenant-details.component.scss"], + templateUrl: "./tenant-details.component.html" +}) +export class TenantDetailsComponent implements OnInit { + public new = false; + public disabled = false; + public tenant!: Tenant; + public tenants = new Array<ResponseTenant>(); + public displayTenant: TreeData; + + constructor(private readonly route: ActivatedRoute, private readonly userService: UserService, + private readonly location: Location) { + this.displayTenant = { + children: [], + id: -1, + name: "" + }; + } + + /** + * Catches when tree-select outputs an update event + * + * @param evt The TreeData selected + */ + public update(evt: TreeData): void { + const tenant = this.tenants.find(t => t.id === evt.id); + if (tenant === undefined) { + console.error(`Unknown tenant selected ${evt.id}`); + return; + } + this.tenant.parentId = tenant.id; + } + + /** + * Recursively fills out a nodes children. + * + * @param tenantByParentId All tenants grouped by parent id. + * @param currentTenant The tenant to populate. + */ + public breakTenantNode(tenantByParentId: Map<number, Array<ResponseTenant>>, currentTenant: TreeData): void { + currentTenant.children = (tenantByParentId.get(currentTenant.id) ?? []).map(t => ({...t, children: []} as TreeData)); + + currentTenant.children.forEach(t => { + this.breakTenantNode(tenantByParentId, t); + }); + } + + /** + * Converts the tenants list into the tree-data structure needed by the tree-select component. + */ + public constructTreeData(): void { + const tenantByParentId = new Map<number, Array<ResponseTenant>>(); + this.tenants.forEach(t => { + if (t.parentId === null) { + return; + } + if (!tenantByParentId.has(t.parentId)) { + tenantByParentId.set(t.parentId, []); + } + // The above check ensures this always returns at least `[]` + // @ts-ignore + tenantByParentId.get(t.parentId).push({...t, children: []}); Review Comment: You can avoid having to use `@ts-ignore` by just doing `tenantByParentId.get` instead of `tenantByParentId.has` and narrowing the returned value's type by checking the null-ish-ness of it. Like so: ```typescript let ts = tenantByParentId.get(t.parentId); if (!ts) { ts = []; tenantByParentId.set(t.parentId, ts); } ts.push({...t, children: []}); ``` ########## experimental/traffic-portal/src/app/shared/tree-select/tree-select.component.scss: ########## @@ -0,0 +1,51 @@ +/* +* 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. +*/ + +div.tree-select-root { + display: block; + position: relative; + + mat-form-field { + width: 100%; + } + + div.tree-select-content { + border: 1px solid black; + position: absolute; + width: 100%; + z-index: 10; + + mat-tree { + ul, li { + margin-top: 0; + margin-bottom: 0; + list-style-type: none; + } + .hidden-node { + display: none; Review Comment: Why is this binding to `class.hidden-node` and applying this style instead of just binding to the `hidden` attribute? ########## experimental/traffic-portal/src/app/core/users/tenants/tenant-details/tenant-details.component.ts: ########## @@ -0,0 +1,175 @@ +/* +* 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 { Location } from "@angular/common"; +import { Component, OnInit } from "@angular/core"; +import { ActivatedRoute } from "@angular/router"; +import { RequestTenant, ResponseTenant, Tenant } from "trafficops-types"; + +import { UserService } from "src/app/api"; +import { TreeData } from "src/app/models/tree-select.model"; + +/** + * TenantsDetailsComponent is the controller for thee tenant add/edit form. + */ +@Component({ + selector: "tp-tenant-details", + styleUrls: ["./tenant-details.component.scss"], + templateUrl: "./tenant-details.component.html" +}) +export class TenantDetailsComponent implements OnInit { + public new = false; + public disabled = false; + public tenant!: Tenant; + public tenants = new Array<ResponseTenant>(); + public displayTenant: TreeData; + + constructor(private readonly route: ActivatedRoute, private readonly userService: UserService, + private readonly location: Location) { + this.displayTenant = { + children: [], + id: -1, + name: "" + }; + } + + /** + * Catches when tree-select outputs an update event + * + * @param evt The TreeData selected + */ + public update(evt: TreeData): void { + const tenant = this.tenants.find(t => t.id === evt.id); + if (tenant === undefined) { + console.error(`Unknown tenant selected ${evt.id}`); + return; + } + this.tenant.parentId = tenant.id; + } + + /** + * Recursively fills out a nodes children. + * + * @param tenantByParentId All tenants grouped by parent id. + * @param currentTenant The tenant to populate. + */ + public breakTenantNode(tenantByParentId: Map<number, Array<ResponseTenant>>, currentTenant: TreeData): void { + currentTenant.children = (tenantByParentId.get(currentTenant.id) ?? []).map(t => ({...t, children: []} as TreeData)); + + currentTenant.children.forEach(t => { + this.breakTenantNode(tenantByParentId, t); + }); + } + + /** + * Converts the tenants list into the tree-data structure needed by the tree-select component. + */ + public constructTreeData(): void { + const tenantByParentId = new Map<number, Array<ResponseTenant>>(); + this.tenants.forEach(t => { + if (t.parentId === null) { + return; + } + if (!tenantByParentId.has(t.parentId)) { + tenantByParentId.set(t.parentId, []); + } + // The above check ensures this always returns at least `[]` + // @ts-ignore + tenantByParentId.get(t.parentId).push({...t, children: []}); + }); + const rootTenant = this.tenants.find(t => t.parentId === null); + if (rootTenant === undefined) { + return; + } + const rootNode = {...rootTenant, children: []} as TreeData; + this.breakTenantNode(tenantByParentId, rootNode); + + this.displayTenant = rootNode; + } + + /** + * Angular lifecycle hook. + */ + public async ngOnInit(): Promise<void> { + const ID = this.route.snapshot.paramMap.get("id"); + if (ID === null) { + console.error("missing required route parameter 'id'"); + return; + } + + this.tenants = await this.userService.getTenants(); + this.constructTreeData(); + + if (ID === "new") { + this.new = true; + this.tenant = { + active: true, + name: "", + } as RequestTenant; + return; + } + const numID = parseInt(ID, 10); + if (Number.isNaN(numID)) { + console.error("route parameter 'id' was non-number:", ID); + return; + } + const tenant = this.tenants.find(t => t.id === numID); + if (!tenant) { + throw new Error(`Unable to find tenant with id ${numID}`); Review Comment: Why does this throw when the other errors are just `console.error`'d? I feel like we should pick one. ########## experimental/traffic-portal/src/app/core/users/tenants/tenants.component.html: ########## @@ -27,4 +27,4 @@ <div id="loading" *ngIf="loading"><tp-loading></tp-loading></div> </mat-card> -<button class="page-fab" mat-fab title="Create a new Tenant" disabled><mat-icon>add</mat-icon></button> +<button class="page-fab" mat-fab title="Create a new Tenant" routerLink="new"><mat-icon>add</mat-icon></button> Review Comment: This button needs to check user Permissions now that it's not always disabled. ########## experimental/traffic-portal/src/app/shared/tree-select/tree-select.component.html: ########## @@ -0,0 +1,47 @@ +<!-- +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. +--> +<div class="tree-select-root" role="combobox"> + <mat-form-field> + <mat-label>{{label}}</mat-label> + <input type="text" id="{{handle}}-tree-select" name="{{handle}}-tree-select" matInput required readonly (click)="toggle($event)" [(ngModel)]="selected.name" [disabled]="disabled"/> + </mat-form-field> + <div class="tree-select-content" *ngIf="shown"> + <mat-form-field appearance="fill" (click)="$event.stopPropagation()"> + <mat-label>Filter</mat-label> + <input type="text" (input)="filterChanged($event)" id="filter-{{handle}}" name="filter-{{handle}}" matInput/> Review Comment: This input should be `type="search"` since that's what it's doing -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
