ocket8888 commented on code in PR #7303:
URL: https://github.com/apache/trafficcontrol/pull/7303#discussion_r1088113905
##########
experimental/traffic-portal/src/app/shared/generic-table/generic-table.component.html:
##########
@@ -49,7 +50,7 @@
<ul>
<li *ngFor="let item of contextMenuItems" role="menuitem">
<ng-container *ngIf="!isAction(item)">
- <a *ngIf="!isDisabled(item)"
[href]="href(item)" [target]="item.newTab ? '_blank' :
'_self'">{{item.name}}</a>
+ <a *ngIf="!isDisabled(item)"
[routerLink]="href(item)" [href]="href(item)" [target]="item.newTab ? '_blank'
: '_self'">{{item.name}}</a>
Review Comment:
`href` and `routerLink` are mutually exclusive. I'm not sure which binding
takes precedence, but they're both binding to the `href` attribute of the
underlying anchor.
##########
experimental/traffic-portal/src/app/api/physical-location.service.ts:
##########
@@ -23,33 +22,75 @@ import { APIService } from "./base-api.service";
*/
@Injectable()
export class PhysicalLocationService extends APIService {
- public async getPhysicalLocations(idOrName: number | string):
Promise<PhysicalLocation>;
- public async getPhysicalLocations(): Promise<Array<PhysicalLocation>>;
+ public async getPhysicalLocations():
Promise<Array<ResponsePhysicalLocation>>;
+ public async getPhysicalLocations(nameOrID: string | number):
Promise<ResponsePhysicalLocation>;
+
/**
- * Gets one or all PhysicalLocations from Traffic Ops
+ * Gets an array of physicalLocations from Traffic Ops.
Review Comment:
The object is `[Response]PhysicalLocation` and the concept it represents is
a "Physical Location," either of which is more appropriate than
`physicalLocation`.
##########
experimental/traffic-portal/src/app/api/physical-location.service.ts:
##########
@@ -23,33 +22,75 @@ import { APIService } from "./base-api.service";
*/
@Injectable()
export class PhysicalLocationService extends APIService {
- public async getPhysicalLocations(idOrName: number | string):
Promise<PhysicalLocation>;
- public async getPhysicalLocations(): Promise<Array<PhysicalLocation>>;
+ public async getPhysicalLocations():
Promise<Array<ResponsePhysicalLocation>>;
+ public async getPhysicalLocations(nameOrID: string | number):
Promise<ResponsePhysicalLocation>;
+
/**
- * Gets one or all PhysicalLocations from Traffic Ops
+ * Gets an array of physicalLocations from Traffic Ops.
*
- * @param idOrName Either the integral, unique identifier (number) or
name (string) of a single PhysicalLocation to be returned.
- * @returns The requested PhysicalLocation(s).
+ * @param nameOrID If given, returns only the PhysicalLocation with the
given name
+ * (string) or ID (number).
+ * @returns An Array of PhysicalLocation objects - or a single
PhysicalLocation object if 'nameOrID'
+ * was given.
*/
- public async getPhysicalLocations(idOrName?: number | string):
Promise<PhysicalLocation | Array<PhysicalLocation>> {
+ public async getPhysicalLocations(nameOrID?: string | number):
Promise<Array<ResponsePhysicalLocation> | ResponsePhysicalLocation> {
const path = "phys_locations";
- let prom;
- if (idOrName !== undefined) {
+ if(nameOrID) {
let params;
- switch (typeof idOrName) {
+ switch (typeof nameOrID) {
case "string":
- params = {name: idOrName};
+ params = {name: nameOrID};
break;
case "number":
- params = {id: String(idOrName)};
+ params = {id: String(nameOrID)};
}
- prom = this.get<[PhysicalLocation]>(path, undefined,
params).toPromise().then(
- r => r[0]
- );
- } else {
- prom =
this.get<Array<PhysicalLocation>>(path).toPromise();
+ const r = await
this.get<[ResponsePhysicalLocation]>(path, undefined, params).toPromise();
+ return {...r[0], lastUpdated: new
Date((r[0].lastUpdated as unknown as string).replace("+00", "Z"))};
+
}
- return prom;
+ const physicalLocations = await
this.get<Array<ResponsePhysicalLocation>>(path).toPromise();
+ return physicalLocations.map(
+ d => ({...d, lastUpdated: new Date((d.lastUpdated as
unknown as string).replace("+00", "Z"))})
+ );
+ }
+
+ /**
+ * Replaces the current definition of a physicalLocation with the one
given.
+ *
+ * @param physicalLocation The new physicalLocation.
+ * @returns The updated physicalLocation.
Review Comment:
The object is `[Response]PhysicalLocation` and the concept it represents is
a "Physical Location," either of which is more appropriate than
`physicalLocation`.
##########
experimental/traffic-portal/src/app/api/physical-location.service.ts:
##########
@@ -23,33 +22,75 @@ import { APIService } from "./base-api.service";
*/
@Injectable()
export class PhysicalLocationService extends APIService {
- public async getPhysicalLocations(idOrName: number | string):
Promise<PhysicalLocation>;
- public async getPhysicalLocations(): Promise<Array<PhysicalLocation>>;
+ public async getPhysicalLocations():
Promise<Array<ResponsePhysicalLocation>>;
+ public async getPhysicalLocations(nameOrID: string | number):
Promise<ResponsePhysicalLocation>;
+
/**
- * Gets one or all PhysicalLocations from Traffic Ops
+ * Gets an array of physicalLocations from Traffic Ops.
*
- * @param idOrName Either the integral, unique identifier (number) or
name (string) of a single PhysicalLocation to be returned.
- * @returns The requested PhysicalLocation(s).
+ * @param nameOrID If given, returns only the PhysicalLocation with the
given name
+ * (string) or ID (number).
+ * @returns An Array of PhysicalLocation objects - or a single
PhysicalLocation object if 'nameOrID'
+ * was given.
*/
- public async getPhysicalLocations(idOrName?: number | string):
Promise<PhysicalLocation | Array<PhysicalLocation>> {
+ public async getPhysicalLocations(nameOrID?: string | number):
Promise<Array<ResponsePhysicalLocation> | ResponsePhysicalLocation> {
const path = "phys_locations";
- let prom;
- if (idOrName !== undefined) {
+ if(nameOrID) {
let params;
- switch (typeof idOrName) {
+ switch (typeof nameOrID) {
case "string":
- params = {name: idOrName};
+ params = {name: nameOrID};
break;
case "number":
- params = {id: String(idOrName)};
+ params = {id: String(nameOrID)};
}
- prom = this.get<[PhysicalLocation]>(path, undefined,
params).toPromise().then(
- r => r[0]
- );
- } else {
- prom =
this.get<Array<PhysicalLocation>>(path).toPromise();
+ const r = await
this.get<[ResponsePhysicalLocation]>(path, undefined, params).toPromise();
+ return {...r[0], lastUpdated: new
Date((r[0].lastUpdated as unknown as string).replace("+00", "Z"))};
+
}
- return prom;
+ const physicalLocations = await
this.get<Array<ResponsePhysicalLocation>>(path).toPromise();
+ return physicalLocations.map(
+ d => ({...d, lastUpdated: new Date((d.lastUpdated as
unknown as string).replace("+00", "Z"))})
+ );
+ }
+
+ /**
+ * Replaces the current definition of a physicalLocation with the one
given.
Review Comment:
The object is `[Response]PhysicalLocation` and the concept it represents is
a "Physical Location," either of which is more appropriate than
`physicalLocation`.
##########
experimental/traffic-portal/src/app/api/testing/physical-location.service.ts:
##########
@@ -12,56 +12,105 @@
* limitations under the License.
*/
import { Injectable } from "@angular/core";
-
-import type { PhysicalLocation } from "src/app/models";
+import { RequestPhysicalLocation, ResponsePhysicalLocation } from
"trafficops-types";
/**
* PhysicalLocationService exposes API functionality relating to
PhysicalLocations.
*/
@Injectable()
export class PhysicalLocationService {
- private readonly locs = [
- {
- address: "1600 Pennsylvania Avenue NW",
- city: "Washington",
- comments: "",
- email: "",
- id: 1,
- lastUpdated: new Date(),
- name: "test",
- phone: "",
- poc: "",
- region: "Washington, D.C",
- regionId: 1,
- shortName: "test",
- state: "DC",
- zip: "20500"
- }
- ];
+ private lastID = 1;
+ private readonly physicalLocations: Array<ResponsePhysicalLocation> = [{
+ address: "street",
+ city: "city",
+ comments: null,
+ email: null,
+ id: 1,
+ lastUpdated: new Date(),
+ name: "phys",
+ phone: null,
+ poc: null,
+ region: "Region",
+ regionId: 1,
+ shortName: "short",
+ state: "st",
+ zip: "0000"
+ }];
+
+ public async getPhysicalLocations():
Promise<Array<ResponsePhysicalLocation>>;
+ public async getPhysicalLocations(nameOrID: string | number):
Promise<ResponsePhysicalLocation>;
- public async getPhysicalLocations(idOrName: number | string):
Promise<PhysicalLocation>;
- public async getPhysicalLocations(): Promise<Array<PhysicalLocation>>;
/**
- * Gets one or all PhysicalLocations from Traffic Ops
+ * Gets one or all of PhysicalLocations from Traffic Ops.
*
- * @param idOrName Either the integral, unique identifier (number) or
name (string) of a single PhysicalLocation to be returned.
- * @returns The requested PhysicalLocation(s).
+ * @param nameOrID If given, returns only the ResponsePhysicalLocation
with the given name
+ * (string) or ID (number).
+ * @returns An Array of ResponsePhysicalLocation objects - or a single
ResponsePhysicalLocation object if 'nameOrID'
+ * was given.
*/
- public async getPhysicalLocations(idOrName?: number | string):
Promise<PhysicalLocation | Array<PhysicalLocation>> {
- if (idOrName !== undefined) {
- let loc;
- switch (typeof idOrName) {
+ public async getPhysicalLocations(nameOrID?: string | number):
Promise<Array<ResponsePhysicalLocation> | ResponsePhysicalLocation> {
+ if(nameOrID) {
+ let physicalLocation;
+ switch (typeof nameOrID) {
case "string":
- loc = this.locs.find(l=>l.name ===
idOrName);
+ physicalLocation =
this.physicalLocations.find(d=>d.name === nameOrID);
break;
case "number":
- loc = this.locs.find(l=>l.id ===
idOrName);
+ physicalLocation =
this.physicalLocations.find(d=>d.id === nameOrID);
}
- if (!loc) {
- throw new Error(`no such Physical Location:
${idOrName}`);
+ if (!physicalLocation) {
+ throw new Error(`no such PhysicalLocation:
${nameOrID}`);
}
- return loc;
+ return physicalLocation;
+ }
+ return this.physicalLocations;
+ }
+
+ /**
+ * Replaces the current definition of a physicalLocation with the one
given.
+ *
+ * @param physicalLocation The new physicalLocation.
+ * @returns The updated physicalLocation.
+ */
+ public async updatePhysicalLocation(physicalLocation:
ResponsePhysicalLocation): Promise<ResponsePhysicalLocation> {
+ const id = this.physicalLocations.findIndex(d => d.id ===
physicalLocation.id);
+ if (id === -1) {
+ throw new Error(`no such PhysicalLocation:
${physicalLocation.id}`);
+ }
+ this.physicalLocations[id] = physicalLocation;
+ return physicalLocation;
+ }
+
+ /**
+ * Creates a new physicalLocation.
+ *
+ * @param physicalLocation The physicalLocation to create.
+ * @returns The created physicalLocation.
+ */
+ public async createPhysicalLocation(physicalLocation:
RequestPhysicalLocation): Promise<ResponsePhysicalLocation> {
+ return {
+ ...physicalLocation,
+ comments: physicalLocation.comments ?? null,
+ email: physicalLocation.email ?? null,
+ id: ++this.lastID,
+ lastUpdated: new Date(),
+ phone: physicalLocation.phone ?? null,
+ poc: physicalLocation.poc ?? null,
+ region: ""
+ };
+ }
Review Comment:
This doesn't appear to actually be inserting the Physical Location into the
testing data
##########
experimental/traffic-portal/src/app/shared/navigation/tp-sidebar/tp-sidebar.component.scss:
##########
@@ -0,0 +1,66 @@
+/*
+ * 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-sidenav-container {
+ height: calc(100% - 3.9em);
+
+ mat-sidenav {
+ min-width: 10em;
+
+ .mat-icon {
+ margin-right: 0;
+ }
+
+
+ div.bottom {
+ position: absolute;
+ bottom: 0;
+ left: 0;
+ width: 100%;
+ }
+
+ .mat-menu-item {
+ width: 100%;
Review Comment:
This width setting is causing a horizontal scrollbar to appear on the opened
sidebar
##########
experimental/traffic-portal/src/app/api/physical-location.service.ts:
##########
@@ -23,33 +22,75 @@ import { APIService } from "./base-api.service";
*/
@Injectable()
export class PhysicalLocationService extends APIService {
- public async getPhysicalLocations(idOrName: number | string):
Promise<PhysicalLocation>;
- public async getPhysicalLocations(): Promise<Array<PhysicalLocation>>;
+ public async getPhysicalLocations():
Promise<Array<ResponsePhysicalLocation>>;
+ public async getPhysicalLocations(nameOrID: string | number):
Promise<ResponsePhysicalLocation>;
+
/**
- * Gets one or all PhysicalLocations from Traffic Ops
+ * Gets an array of physicalLocations from Traffic Ops.
*
- * @param idOrName Either the integral, unique identifier (number) or
name (string) of a single PhysicalLocation to be returned.
- * @returns The requested PhysicalLocation(s).
+ * @param nameOrID If given, returns only the PhysicalLocation with the
given name
+ * (string) or ID (number).
+ * @returns An Array of PhysicalLocation objects - or a single
PhysicalLocation object if 'nameOrID'
+ * was given.
*/
- public async getPhysicalLocations(idOrName?: number | string):
Promise<PhysicalLocation | Array<PhysicalLocation>> {
+ public async getPhysicalLocations(nameOrID?: string | number):
Promise<Array<ResponsePhysicalLocation> | ResponsePhysicalLocation> {
const path = "phys_locations";
- let prom;
- if (idOrName !== undefined) {
+ if(nameOrID) {
let params;
- switch (typeof idOrName) {
+ switch (typeof nameOrID) {
case "string":
- params = {name: idOrName};
+ params = {name: nameOrID};
break;
case "number":
- params = {id: String(idOrName)};
+ params = {id: String(nameOrID)};
}
- prom = this.get<[PhysicalLocation]>(path, undefined,
params).toPromise().then(
- r => r[0]
- );
- } else {
- prom =
this.get<Array<PhysicalLocation>>(path).toPromise();
+ const r = await
this.get<[ResponsePhysicalLocation]>(path, undefined, params).toPromise();
+ return {...r[0], lastUpdated: new
Date((r[0].lastUpdated as unknown as string).replace("+00", "Z"))};
+
}
- return prom;
+ const physicalLocations = await
this.get<Array<ResponsePhysicalLocation>>(path).toPromise();
+ return physicalLocations.map(
+ d => ({...d, lastUpdated: new Date((d.lastUpdated as
unknown as string).replace("+00", "Z"))})
+ );
+ }
+
+ /**
+ * Replaces the current definition of a physicalLocation with the one
given.
+ *
+ * @param physicalLocation The new physicalLocation.
+ * @returns The updated physicalLocation.
+ */
+ public async updatePhysicalLocation(physicalLocation:
ResponsePhysicalLocation): Promise<ResponsePhysicalLocation> {
+ const path = `phys_locations/${physicalLocation.id}`;
+ const response = await this.put<ResponsePhysicalLocation>(path,
physicalLocation).toPromise();
+ return {
+ ...response,
+ lastUpdated: new Date((response.lastUpdated as unknown
as string).replace(" ", "T").replace("+00", "Z"))
+ };
+ }
+
+ /**
+ * Creates a new physicalLocation.
+ *
+ * @param physicalLocation The physicalLocation to create.
+ * @returns The created physicalLocation.
+ */
+ public async createPhysicalLocation(physicalLocation:
RequestPhysicalLocation): Promise<ResponsePhysicalLocation> {
+ const response = await
this.post<ResponsePhysicalLocation>("physicalLocations",
physicalLocation).toPromise();
+ return {
+ ...response,
+ lastUpdated: new Date((response.lastUpdated as unknown
as string).replace(" ", "T").replace("+00", "Z"))
+ };
+ }
+
+ /**
+ * Deletes an existing physicalLocation.
+ *
+ * @param physLoc The Physical Location to be deleted (or its ID)
+ */
Review Comment:
The object is `[Response]PhysicalLocation` and the concept it represents is
a "Physical Location," either of which is more appropriate than
`physicalLocation`.
##########
experimental/traffic-portal/src/app/api/physical-location.service.ts:
##########
@@ -23,33 +22,75 @@ import { APIService } from "./base-api.service";
*/
@Injectable()
export class PhysicalLocationService extends APIService {
- public async getPhysicalLocations(idOrName: number | string):
Promise<PhysicalLocation>;
- public async getPhysicalLocations(): Promise<Array<PhysicalLocation>>;
+ public async getPhysicalLocations():
Promise<Array<ResponsePhysicalLocation>>;
+ public async getPhysicalLocations(nameOrID: string | number):
Promise<ResponsePhysicalLocation>;
+
/**
- * Gets one or all PhysicalLocations from Traffic Ops
+ * Gets an array of physicalLocations from Traffic Ops.
*
- * @param idOrName Either the integral, unique identifier (number) or
name (string) of a single PhysicalLocation to be returned.
- * @returns The requested PhysicalLocation(s).
+ * @param nameOrID If given, returns only the PhysicalLocation with the
given name
+ * (string) or ID (number).
+ * @returns An Array of PhysicalLocation objects - or a single
PhysicalLocation object if 'nameOrID'
+ * was given.
*/
- public async getPhysicalLocations(idOrName?: number | string):
Promise<PhysicalLocation | Array<PhysicalLocation>> {
+ public async getPhysicalLocations(nameOrID?: string | number):
Promise<Array<ResponsePhysicalLocation> | ResponsePhysicalLocation> {
const path = "phys_locations";
- let prom;
- if (idOrName !== undefined) {
+ if(nameOrID) {
let params;
- switch (typeof idOrName) {
+ switch (typeof nameOrID) {
case "string":
- params = {name: idOrName};
+ params = {name: nameOrID};
break;
case "number":
- params = {id: String(idOrName)};
+ params = {id: String(nameOrID)};
}
- prom = this.get<[PhysicalLocation]>(path, undefined,
params).toPromise().then(
- r => r[0]
- );
- } else {
- prom =
this.get<Array<PhysicalLocation>>(path).toPromise();
+ const r = await
this.get<[ResponsePhysicalLocation]>(path, undefined, params).toPromise();
+ return {...r[0], lastUpdated: new
Date((r[0].lastUpdated as unknown as string).replace("+00", "Z"))};
+
}
- return prom;
+ const physicalLocations = await
this.get<Array<ResponsePhysicalLocation>>(path).toPromise();
+ return physicalLocations.map(
+ d => ({...d, lastUpdated: new Date((d.lastUpdated as
unknown as string).replace("+00", "Z"))})
+ );
+ }
+
+ /**
+ * Replaces the current definition of a physicalLocation with the one
given.
+ *
+ * @param physicalLocation The new physicalLocation.
+ * @returns The updated physicalLocation.
+ */
+ public async updatePhysicalLocation(physicalLocation:
ResponsePhysicalLocation): Promise<ResponsePhysicalLocation> {
+ const path = `phys_locations/${physicalLocation.id}`;
+ const response = await this.put<ResponsePhysicalLocation>(path,
physicalLocation).toPromise();
+ return {
+ ...response,
+ lastUpdated: new Date((response.lastUpdated as unknown
as string).replace(" ", "T").replace("+00", "Z"))
+ };
+ }
+
+ /**
+ * Creates a new physicalLocation.
+ *
+ * @param physicalLocation The physicalLocation to create.
+ * @returns The created physicalLocation.
+ */
Review Comment:
The object is `[Response]PhysicalLocation` and the concept it represents is
a "Physical Location," either of which is more appropriate than
`physicalLocation`.
--
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]
