ocket8888 commented on code in PR #7881: URL: https://github.com/apache/trafficcontrol/pull/7881#discussion_r1416060002
########## experimental/traffic-portal/src/app/api/origin.service.ts: ########## @@ -0,0 +1,128 @@ +/* +* 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 {RequestOrigin, ResponseOrigin} from "trafficops-types/dist/origin"; + +import { APIService } from "./base-api.service"; + +/** The allowed values for the 'useInTables' query parameter of GET requests to /origins. */ +type UseInTable = "cachegroup" | +"server" | +"deliveryservice" | +"to_extension" | +"federation_resolver" | +"regex" | +"staticdnsentry" | +"steering_target"; + +/** + * OriginService exposes API functionality relating to Origins. + */ +@Injectable() +export class OriginService extends APIService { + /** + * Gets a specific Origin from Traffic Ops. + * + * @param idOrName Either the integral, unique identifier (number) or name + * (string) of the Origin to be returned. + * @returns The requested Origin. + */ + public async getOrigins(idOrName: number | string): Promise<ResponseOrigin>; + /** + * Gets Origins from Traffic Ops. + * + * @param idOrName Either the integral, unique identifier (number) or name + * (string) of a single Origin to be returned. + * @returns The requested Origin(s). + */ + public async getOrigins(): Promise<Array<ResponseOrigin>>; + /** + * Gets one or all Origins from Traffic Ops. + * + * @param idOrName Optionally the integral, unique identifier (number) or + * name (string) of a single Origin to be returned. + * @returns The requested Origin(s). + */ + public async getOrigins(idOrName?: number | string): Promise<ResponseOrigin | Array<ResponseOrigin>> { + const path = "origins"; + if (idOrName !== undefined) { + let params; + switch (typeof idOrName) { + case "string": + params = {name: idOrName}; + break; + case "number": + params = {id: idOrName}; + } + const r = await this.get<[ResponseOrigin]>(path, undefined, params).toPromise(); + if (r.length !== 1) { + throw new Error(`Traffic Ops responded with ${r.length} Origins by identifier ${idOrName}`); + } + return r[0]; + } + return this.get<Array<ResponseOrigin>>(path).toPromise(); + } + + /** + * Gets all Origins used by specific database table. + * + * @param useInTable The database table for which to retrieve Origins. + * @returns The requested Origins. + */ + public async getOriginsInTable(useInTable: UseInTable): Promise<Array<ResponseOrigin>> { Review Comment: This method shouldn't exist, it doesn't appear to actually relate to Origins at all. ########## experimental/traffic-portal/src/app/api/origin.service.ts: ########## @@ -0,0 +1,128 @@ +/* +* 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 {RequestOrigin, ResponseOrigin} from "trafficops-types/dist/origin"; + +import { APIService } from "./base-api.service"; + +/** The allowed values for the 'useInTables' query parameter of GET requests to /origins. */ +type UseInTable = "cachegroup" | Review Comment: you shouldn't need this type ########## experimental/traffic-portal/src/app/api/origin.service.ts: ########## @@ -0,0 +1,128 @@ +/* +* 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 {RequestOrigin, ResponseOrigin} from "trafficops-types/dist/origin"; Review Comment: nit: import could be `import type` since all imported symbols are only used as types. ########## experimental/traffic-portal/src/app/core/origins/detail/origin-detail.component.html: ########## @@ -0,0 +1,49 @@ +<!-- +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="!type"></tp-loading> + <form ngNativeValidate (ngSubmit)="submit($event)" *ngIf="type"> + <mat-card-header class="headers-container" *ngIf="!new"> + <a mat-raised-button color="primary" *ngIf="type.useInTable === 'cachegroup'" [routerLink]="'/core/cache-groups'" [queryParams]="{typeName: type.name}">View Cache Groups</a> + <a mat-raised-button color="primary" *ngIf="type.useInTable === 'server'" [routerLink]="'/core/servers'" [queryParams]="{type: type.name}">View Servers</a> Review Comment: These links don't make sense, since an origin cannot be used to group servers or cache groups ########## experimental/traffic-portal/src/app/core/origins/detail/origin-detail.component.html: ########## @@ -0,0 +1,49 @@ +<!-- +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="!type"></tp-loading> + <form ngNativeValidate (ngSubmit)="submit($event)" *ngIf="type"> + <mat-card-header class="headers-container" *ngIf="!new"> + <a mat-raised-button color="primary" *ngIf="type.useInTable === 'cachegroup'" [routerLink]="'/core/cache-groups'" [queryParams]="{typeName: type.name}">View Cache Groups</a> + <a mat-raised-button color="primary" *ngIf="type.useInTable === 'server'" [routerLink]="'/core/servers'" [queryParams]="{type: type.name}">View Servers</a> + </mat-card-header> + <mat-card-content class="container"> + <mat-form-field *ngIf="!new"> + <mat-label>ID</mat-label> + <input matInput type="text" name="id" disabled readonly [defaultValue]="type.id" /> + </mat-form-field> + <mat-form-field> + <mat-label>Name</mat-label> + <input matInput type="text" name="name" required [(ngModel)]="type.name" /> + </mat-form-field> + <mat-form-field> + <mat-label>Description</mat-label> + <input matInput type="text" name="description" required [(ngModel)]="type.description" /> + </mat-form-field> + <mat-form-field> + <mat-label>Use In Table</mat-label> + <input matInput type="text" name="useInTable" disabled readonly [defaultValue]="new ? 'server' : type.useInTable" /> + </mat-form-field> + <mat-form-field *ngIf="!new"> + <mat-label>Last Updated</mat-label> + <input matInput type="text" name="lastUpdated" disabled readonly [defaultValue]="type.lastUpdated" /> + </mat-form-field> + </mat-card-content> Review Comment: These fields should reflect properties of an Origin, currently they appear to represent a Type ########## experimental/traffic-portal/src/app/core/origins/table/origins-table.component.ts: ########## @@ -0,0 +1,147 @@ +/* +* 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 { Component, type OnInit } from "@angular/core"; +import { FormControl } from "@angular/forms"; +import { MatDialog } from "@angular/material/dialog"; +import { ActivatedRoute } from "@angular/router"; +import { BehaviorSubject } from "rxjs"; +import { TypeFromResponse } from "trafficops-types"; +import { ResponseOrigin } from "trafficops-types/dist/origin"; + +import { OriginService } from "src/app/api"; +import { CurrentUserService } from "src/app/shared/current-user/current-user.service"; +import { DecisionDialogComponent } from "src/app/shared/dialogs/decision-dialog/decision-dialog.component"; +import type { + ContextMenuActionEvent, + ContextMenuItem, + DoubleClickLink +} from "src/app/shared/generic-table/generic-table.component"; +import { NavigationService } from "src/app/shared/navigation/navigation.service"; + +/** + * OriginsTableComponent is the controller for the "Origins" table. + */ +@Component({ + selector: "tp-origins", + styleUrls: ["./origins-table.component.scss"], + templateUrl: "./origins-table.component.html" +}) +export class OriginsTableComponent implements OnInit { + /** List of origins */ + public origins: Promise<Array<ResponseOrigin>>; + + /** Definitions of the table's columns according to the ag-grid API */ + public columnDefs = [ + { + field: "name", + headerName: "Name" + }, + { + field: "tenant", + headerName: "Tenant" + }, + { + field: "primary", + headerName: "Primary" Review Comment: this field should use the `tpBooleanFilter` filter ########## experimental/traffic-portal/src/app/core/origins/detail/origin-detail.component.spec.ts: ########## @@ -0,0 +1,78 @@ +/* +* 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 { TypeDetailComponent } from "src/app/core/types/detail/type-detail.component"; +import { NavigationService } from "src/app/shared/navigation/navigation.service"; + +describe("TypeDetailComponent", () => { Review Comment: This needs to be overhauled, it would seem ########## experimental/traffic-portal/src/app/api/origin.service.ts: ########## @@ -0,0 +1,128 @@ +/* +* 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 {RequestOrigin, ResponseOrigin} from "trafficops-types/dist/origin"; + +import { APIService } from "./base-api.service"; + +/** The allowed values for the 'useInTables' query parameter of GET requests to /origins. */ +type UseInTable = "cachegroup" | +"server" | +"deliveryservice" | +"to_extension" | +"federation_resolver" | +"regex" | +"staticdnsentry" | +"steering_target"; + +/** + * OriginService exposes API functionality relating to Origins. + */ +@Injectable() +export class OriginService extends APIService { + /** + * Gets a specific Origin from Traffic Ops. + * + * @param idOrName Either the integral, unique identifier (number) or name + * (string) of the Origin to be returned. + * @returns The requested Origin. + */ + public async getOrigins(idOrName: number | string): Promise<ResponseOrigin>; + /** + * Gets Origins from Traffic Ops. + * + * @param idOrName Either the integral, unique identifier (number) or name + * (string) of a single Origin to be returned. + * @returns The requested Origin(s). + */ + public async getOrigins(): Promise<Array<ResponseOrigin>>; + /** + * Gets one or all Origins from Traffic Ops. + * + * @param idOrName Optionally the integral, unique identifier (number) or + * name (string) of a single Origin to be returned. + * @returns The requested Origin(s). + */ + public async getOrigins(idOrName?: number | string): Promise<ResponseOrigin | Array<ResponseOrigin>> { + const path = "origins"; + if (idOrName !== undefined) { + let params; + switch (typeof idOrName) { + case "string": + params = {name: idOrName}; + break; + case "number": + params = {id: idOrName}; + } + const r = await this.get<[ResponseOrigin]>(path, undefined, params).toPromise(); + if (r.length !== 1) { + throw new Error(`Traffic Ops responded with ${r.length} Origins by identifier ${idOrName}`); + } + return r[0]; + } + return this.get<Array<ResponseOrigin>>(path).toPromise(); + } + + /** + * Gets all Origins used by specific database table. + * + * @param useInTable The database table for which to retrieve Origins. + * @returns The requested Origins. + */ + public async getOriginsInTable(useInTable: UseInTable): Promise<Array<ResponseOrigin>> { + return this.get<Array<ResponseOrigin>>("types", undefined, {useInTable}).toPromise(); + } + + /** + * Deletes an existing origin. + * + * @param typeOrId Id of the origin to delete. + * @returns The deleted origin. + */ + public async deleteOrigin(typeOrId: number | ResponseOrigin): Promise<ResponseOrigin> { + const id = typeof(typeOrId) === "number" ? typeOrId : typeOrId.id; + return this.delete<ResponseOrigin>(`origins/${id}`).toPromise(); + } + + /** + * Creates a new origin. + * + * @param origin The origin to create. + * @returns The created origin. + */ + public async createOrigin(origin: RequestOrigin): Promise<ResponseOrigin> { + return this.post<ResponseOrigin>("origins", origin).toPromise(); + } + + /** + * Replaces the current definition of a origin with the one given. + * + * @param origin The new origin. + * @returns The updated origin. + */ + public async updateOrigin(origin: ResponseOrigin): Promise<ResponseOrigin> { + const path = `origins/${origin.id}`; + return this.put<ResponseOrigin>(path, origin).toPromise(); + } + + /** + * Injects the Angular HTTP client service into the parent constructor. + * + * @param http The Angular HTTP client service. + */ + constructor(http: HttpClient) { + super(http); + } +} Review Comment: this file should have an associated testing service as well as tests. ########## experimental/traffic-portal/src/app/core/origins/table/origins-table.component.ts: ########## @@ -0,0 +1,147 @@ +/* +* 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 { Component, type OnInit } from "@angular/core"; +import { FormControl } from "@angular/forms"; +import { MatDialog } from "@angular/material/dialog"; +import { ActivatedRoute } from "@angular/router"; +import { BehaviorSubject } from "rxjs"; +import { TypeFromResponse } from "trafficops-types"; +import { ResponseOrigin } from "trafficops-types/dist/origin"; + +import { OriginService } from "src/app/api"; +import { CurrentUserService } from "src/app/shared/current-user/current-user.service"; +import { DecisionDialogComponent } from "src/app/shared/dialogs/decision-dialog/decision-dialog.component"; +import type { + ContextMenuActionEvent, + ContextMenuItem, + DoubleClickLink +} from "src/app/shared/generic-table/generic-table.component"; +import { NavigationService } from "src/app/shared/navigation/navigation.service"; + +/** + * OriginsTableComponent is the controller for the "Origins" table. + */ +@Component({ + selector: "tp-origins", + styleUrls: ["./origins-table.component.scss"], + templateUrl: "./origins-table.component.html" +}) +export class OriginsTableComponent implements OnInit { + /** List of origins */ + public origins: Promise<Array<ResponseOrigin>>; + + /** Definitions of the table's columns according to the ag-grid API */ + public columnDefs = [ + { + field: "name", + headerName: "Name" + }, + { + field: "tenant", + headerName: "Tenant" Review Comment: you should use a valueGetter or something to add in the ID of the Tenant. Unfortunately, that's still sometimes more important to know than the name. ########## experimental/traffic-portal/src/app/core/origins/detail/origin-detail.component.ts: ########## @@ -0,0 +1,116 @@ +/* +* 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 { MatDialog } from "@angular/material/dialog"; +import { ActivatedRoute } from "@angular/router"; +import { TypeFromResponse } from "trafficops-types"; + +import { TypeService } from "src/app/api"; +import { DecisionDialogComponent } from "src/app/shared/dialogs/decision-dialog/decision-dialog.component"; +import { LoggingService } from "src/app/shared/logging.service"; +import { NavigationService } from "src/app/shared/navigation/navigation.service"; + +/** + * TypeDetailsComponent is the controller for the type add/edit form. Review Comment: I guess I should just stop commenting on this, but it seems like this file also needs to be overhauled as it still looks copy/pasted from somewhere else. ########## experimental/traffic-portal/src/app/core/origins/table/origins-table.component.ts: ########## @@ -0,0 +1,147 @@ +/* +* 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 { Component, type OnInit } from "@angular/core"; +import { FormControl } from "@angular/forms"; +import { MatDialog } from "@angular/material/dialog"; +import { ActivatedRoute } from "@angular/router"; +import { BehaviorSubject } from "rxjs"; +import { TypeFromResponse } from "trafficops-types"; +import { ResponseOrigin } from "trafficops-types/dist/origin"; + +import { OriginService } from "src/app/api"; +import { CurrentUserService } from "src/app/shared/current-user/current-user.service"; +import { DecisionDialogComponent } from "src/app/shared/dialogs/decision-dialog/decision-dialog.component"; +import type { + ContextMenuActionEvent, + ContextMenuItem, + DoubleClickLink +} from "src/app/shared/generic-table/generic-table.component"; +import { NavigationService } from "src/app/shared/navigation/navigation.service"; + +/** + * OriginsTableComponent is the controller for the "Origins" table. + */ +@Component({ + selector: "tp-origins", + styleUrls: ["./origins-table.component.scss"], + templateUrl: "./origins-table.component.html" +}) +export class OriginsTableComponent implements OnInit { + /** List of origins */ + public origins: Promise<Array<ResponseOrigin>>; + + /** Definitions of the table's columns according to the ag-grid API */ + public columnDefs = [ + { + field: "name", + headerName: "Name" + }, + { + field: "tenant", + headerName: "Tenant" + }, + { + field: "primary", + headerName: "Primary" + }, + { + field: "delivery-service", + headerName: "Delivery Service" + }, + { + field: "fqdn", + headerName: "FQDN" + }, + { + field: "lastUpdated", + headerName: "Last Updated" Review Comment: this should use a date filter -- 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]
