ocket8888 commented on code in PR #7480:
URL: https://github.com/apache/trafficcontrol/pull/7480#discussion_r1198258283


##########
experimental/traffic-portal/src/app/api/profile.service.ts:
##########
@@ -71,6 +71,27 @@ export class ProfileService extends APIService {
                return this.get<Array<ResponseProfile>>(path).toPromise();
        }
 
+       /**
+        * Retrieves Profiles associated with a Parameter from the API.
+        *
+        * @param p Either a {@link ResponseParameter} or an integral, unique 
identifier of a Parameter, for which the
+        * Profiles are to be retrieved.
+        * @returns The requested Profile(s).
+        */
+       public async getProfilesByParam(p: number| ResponseParameter): 
Promise<Array<ResponseProfile>> {

Review Comment:
   This method is not covered by tests



##########
experimental/traffic-portal/src/app/api/profile.service.ts:
##########
@@ -103,4 +124,64 @@ export class ProfileService extends APIService {
                return 
this.delete<ResponseProfile>(`profiles/${id}`).toPromise();
        }
 
+       public async getParameters(idOrName: number | string): 
Promise<ResponseParameter>;
+       public async getParameters(): Promise<Array<ResponseParameter>>;
+       /**
+        * Retrieves Parameters from the API.
+        *
+        * @param idOrName Specify either the integral, unique identifier 
(number) of a specific Parameter to retrieve,
+        * or its name (string).
+        * @returns The requested Parameter(s).
+        */
+       public async getParameters(idOrName?: number | string): 
Promise<Array<ResponseParameter> | ResponseParameter> {
+               const path = "parameters";
+               if (idOrName !== undefined) {
+                       let params;
+                       switch (typeof idOrName) {
+                               case "number":
+                                       params = {id: idOrName};
+                                       break;
+                               case "string":
+                                       params = {name: idOrName};
+                       }
+                       const r = await this.get<[ResponseParameter]>(path, 
undefined, params).toPromise();
+                       if (r.length !== 1) {
+                               throw new Error(`Traffic Ops responded with 
${r.length} Types by identifier ${idOrName}`);

Review Comment:
   The tests for `getParameters` only cover happy paths; this line - the case 
where more than one Parameter exists with the given name or ID - is not covered.



##########
experimental/traffic-portal/src/app/api/profile.service.ts:
##########
@@ -103,4 +124,64 @@ export class ProfileService extends APIService {
                return 
this.delete<ResponseProfile>(`profiles/${id}`).toPromise();
        }
 
+       public async getParameters(idOrName: number | string): 
Promise<ResponseParameter>;
+       public async getParameters(): Promise<Array<ResponseParameter>>;
+       /**
+        * Retrieves Parameters from the API.
+        *
+        * @param idOrName Specify either the integral, unique identifier 
(number) of a specific Parameter to retrieve,
+        * or its name (string).
+        * @returns The requested Parameter(s).
+        */
+       public async getParameters(idOrName?: number | string): 
Promise<Array<ResponseParameter> | ResponseParameter> {

Review Comment:
   Parameters cannot be uniquely identified by name. Any number of Parameters 
can - and do, for the vast majority of Parameters - exist by the same name at 
any given time. So if you want to be able to filter Parameters by name that's 
fine, but neither this method itself nor the caller should expect that doing so 
will return a single Parameter.



-- 
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]

Reply via email to