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


##########
experimental/traffic-portal/src/app/api/user.service.ts:
##########
@@ -267,6 +268,38 @@ export class UserService extends APIService {
                return this.get<Array<ResponseRole>>(path).toPromise();
        }
 
+       /**
+        * Creates a new Role.
+        *
+        * @param role The role to create.
+        * @returns The created role along with lastUpdated field.
+        */
+       public async createRole(role: RequestRole): Promise<ResponseRole> {
+               return this.post<ResponseRole>("roles", role).toPromise();
+       }
+
+       /**
+        * Updates an existing Role.
+        *
+        * @param name The original role name
+        * @param role The role to update.
+        * @returns The updated role without lastUpdated field.
+        */
+       public async updateRole(name: string, role: ResponseRole): 
Promise<ResponseRole> {
+               return this.put<ResponseRole>("roles?", role, 
{name}).toPromise();
+       }
+
+       /**
+        * Deletes an existing role.
+        *
+        * @param role The role to be deleted.
+        * @returns The deleted role.
+        */
+       public async deleteRole(role: string | ResponseRole): Promise<void> {
+               const name = typeof(role) === "string" ? role : role.name;
+               return this.delete("roles", undefined, {name}).toPromise();
+       }
+

Review Comment:
   missing tests for these API methods



##########
experimental/traffic-portal/src/app/core/users/roles/detail/role-detail.component.html:
##########
@@ -0,0 +1,37 @@
+<!--
+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>
+       <tp-loading *ngIf="!role"></tp-loading>
+       <form ngNativeValidate (ngSubmit)="submit($event)" *ngIf="role">
+               <mat-card-content>
+                       <mat-form-field>
+                               <mat-label>Name</mat-label>
+                               <input matInput type="text" name="name" 
required [(ngModel)]="role.name" [readonly]="role.name === 'admin'"/>

Review Comment:
   instead of `readonly`, a better ux would be to make these `disabled`; if 
it's `readonly` the control still _appears_ usable, but just doesn't actually 
allow you to use it. `disabled` applies styling that makes it clear at a glance 
that it can't be edited.



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

Reply via email to