This is an automated email from the ASF dual-hosted git repository. rshah pushed a commit to branch feature/tpv2-role-details in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
commit 2b172108f9c77bc263440fcb7637e2da180152eb Author: Rima Shah <[email protected]> AuthorDate: Wed May 10 23:31:57 2023 -0600 Corrections wrt tests --- .../users/{roleDetails.ts => roleDetail.ts} | 3 -- .../nightwatch/tests/users/role/detail.spec.ts | 32 ++++++++------- .../src/app/api/testing/user.service.ts | 30 +++++++++------ .../traffic-portal/src/app/api/user.service.ts | 9 +++-- .../traffic-portal/src/app/core/core.module.ts | 1 + .../users/roles/detail/role-detail.component.html | 19 ++++----- .../roles/detail/role-detail.component.spec.ts | 19 ++++----- .../users/roles/detail/role-detail.component.ts | 20 +++++----- .../roles/table/roles-table.component.spec.ts | 45 ++++++++++++++++------ .../users/roles/table/roles-table.component.ts | 32 ++++++++++++--- 10 files changed, 124 insertions(+), 86 deletions(-) diff --git a/experimental/traffic-portal/nightwatch/page_objects/users/roleDetails.ts b/experimental/traffic-portal/nightwatch/page_objects/users/roleDetail.ts similarity index 94% rename from experimental/traffic-portal/nightwatch/page_objects/users/roleDetails.ts rename to experimental/traffic-portal/nightwatch/page_objects/users/roleDetail.ts index de01fda53d..c4b0521b25 100644 --- a/experimental/traffic-portal/nightwatch/page_objects/users/roleDetails.ts +++ b/experimental/traffic-portal/nightwatch/page_objects/users/roleDetail.ts @@ -24,9 +24,6 @@ const roleDetailPageObject = { description: { selector: "input[name='description']" }, - lastUpdated: { - selector: "input[name='lastUpdated']" - }, name: { selector: "input[name='name']" }, diff --git a/experimental/traffic-portal/nightwatch/tests/users/role/detail.spec.ts b/experimental/traffic-portal/nightwatch/tests/users/role/detail.spec.ts index 8856106316..95d9d2a9ad 100644 --- a/experimental/traffic-portal/nightwatch/tests/users/role/detail.spec.ts +++ b/experimental/traffic-portal/nightwatch/tests/users/role/detail.spec.ts @@ -14,31 +14,29 @@ describe("Role Detail Spec", () => { it("Test Role", () => { - const page = browser.page.users.roles(); + const page = browser.page.users.roleDetail(); browser.url(`${page.api.launchUrl}/core/roles/${browser.globals.testData.role.name}`, res => { browser.assert.ok(res.status === 0); page.waitForElementVisible("mat-card") - .assert.enabled("@role") .assert.enabled("@description") + .assert.enabled("@name") .assert.enabled("@permissions") .assert.enabled("@saveBtn") - .assert.not.enabled("@lastUpdated") .assert.valueEquals("@name", String(browser.globals.testData.role.name)) - .assert.valueEquals("@permission", String(browser.globals.testData.role.permissions)); + .assert.valueEquals("@permissions", String(browser.globals.testData.role.permissions)); }); }); - // it("New asn", () => { - // const page = browser.page.cacheGroups.asnDetail(); - // browser.url(`${page.api.launchUrl}/core/asns/new`, res => { - // browser.assert.ok(res.status === 0); - // page.waitForElementVisible("mat-card") - // .assert.enabled("@asn") - // .assert.enabled("@cachegroup") - // .assert.enabled("@saveBtn") - // .assert.not.elementPresent("@id") - // .assert.not.elementPresent("@lastUpdated") - // .assert.valueEquals("@asn", "1"); - // }); - // }); + it("New asn", () => { + const page = browser.page.users.roleDetail(); + browser.url(`${page.api.launchUrl}/core/roles/new`, res => { + browser.assert.ok(res.status === 0); + page.waitForElementVisible("mat-card") + .assert.enabled("@description") + .assert.enabled("@name") + .assert.enabled("@permissions") + .assert.enabled("@saveBtn") + .assert.valueEquals("@name", "test"); + }); + }); }); diff --git a/experimental/traffic-portal/src/app/api/testing/user.service.ts b/experimental/traffic-portal/src/app/api/testing/user.service.ts index bcaf7c6dc2..e176d4ce9f 100644 --- a/experimental/traffic-portal/src/app/api/testing/user.service.ts +++ b/experimental/traffic-portal/src/app/api/testing/user.service.ts @@ -86,6 +86,15 @@ export class UserService { privLevel: 30 } ]; + private readonly roleDetail: Array<ResponseRole> = [{ + description: "Has access to everything - cannot be modified or deleted", + lastUpdated: new Date(), + name: "admin", + permissions: [ + "ALL" + ], + } + ]; private readonly tenants: Array<ResponseTenant> = [ { @@ -438,10 +447,8 @@ export class UserService { public async createRole(role: RequestRole): Promise<ResponseRole> { const resp = { ...role, - name: role.name, - lastUpdated: new Date(), }; - this.roles.push(resp); + this.roleDetail.push(resp); return resp; } @@ -452,11 +459,11 @@ export class UserService { * @returns The updated role. */ public async updateRole(role: ResponseRole): Promise<ResponseRole> { - const name = this.tenants.findIndex(r => r.name === role.name); - if (name === null) { + const roleName = this.roleDetail.findIndex(r => r.name === role.name); + if (roleName < 0 ) { throw new Error(`no such Role: ${role.name}`); } - this.roles[name] = role; + this.roleDetail[roleName] = role; return role; } @@ -466,12 +473,13 @@ export class UserService { * @param tenant The role to be deleted. * @returns The deleted role. */ - public async deleteRole(role: ResponseRole): Promise<ResponseRole> { - const index = this.tenants.findIndex(r => r.name === role.name); - if (index < 0) { - throw new Error(`no such role: ${role.name}`); + public async deleteRole(role: string | ResponseRole): Promise<ResponseRole> { + const roleName = typeof(role) === "string" ? role : role.name; + const index = this.roleDetail.findIndex(r => r.name === roleName); + if (index === -1) { + throw new Error(`no such role: ${role}`); } - return this.roles.splice(index, 1)[0]; + return this.roleDetail.splice(index, 1)[0]; } /** diff --git a/experimental/traffic-portal/src/app/api/user.service.ts b/experimental/traffic-portal/src/app/api/user.service.ts index 3b1bb51b1c..4a1ad356e9 100644 --- a/experimental/traffic-portal/src/app/api/user.service.ts +++ b/experimental/traffic-portal/src/app/api/user.service.ts @@ -17,6 +17,7 @@ import { Injectable } from "@angular/core"; import { type ResponseUser, type PostRequestUser, + type RequestRole, type RequestTenant, type ResponseCurrentUser, type ResponseRole, @@ -27,7 +28,6 @@ import { } from "trafficops-types"; import { APIService } from "./base-api.service"; -import {RequestRole} from "trafficops-types"; /** * UserService exposes API functionality related to Users, Roles and Tenants. @@ -285,7 +285,7 @@ export class UserService extends APIService { * @returns The updated role. */ public async updateRole(role: ResponseRole): Promise<ResponseRole> { - return this.put<ResponseRole>(`roles/${role.name}`, role).toPromise(); + return this.put<ResponseRole>(`roles?name=${role.name}`, role).toPromise(); } /** @@ -294,8 +294,9 @@ export class UserService extends APIService { * @param tenant The role to be deleted. * @returns The deleted role. */ - public async deleteRole(role: ResponseRole): Promise<ResponseRole> { - return this.delete<ResponseRole>(`roles/${role.name}`).toPromise(); + public async deleteRole(role: string | ResponseRole): Promise<void> { + const roleName = typeof(role) === "string" ? role : role.name; + return this.delete(`roles?name=${roleName}`).toPromise(); } /** diff --git a/experimental/traffic-portal/src/app/core/core.module.ts b/experimental/traffic-portal/src/app/core/core.module.ts index f90d850688..e9b68f3cd6 100644 --- a/experimental/traffic-portal/src/app/core/core.module.ts +++ b/experimental/traffic-portal/src/app/core/core.module.ts @@ -131,6 +131,7 @@ export const ROUTES: Routes = [ TenantsComponent, UserRegistrationDialogComponent, RolesTableComponent, + RoleDetailComponent, TenantDetailsComponent, ChangeLogsComponent, LastDaysComponent, diff --git a/experimental/traffic-portal/src/app/core/users/roles/detail/role-detail.component.html b/experimental/traffic-portal/src/app/core/users/roles/detail/role-detail.component.html index 512420ec62..9a46d242fb 100644 --- a/experimental/traffic-portal/src/app/core/users/roles/detail/role-detail.component.html +++ b/experimental/traffic-portal/src/app/core/users/roles/detail/role-detail.component.html @@ -16,23 +16,18 @@ limitations under the License. <tp-loading *ngIf="!role"></tp-loading> <form ngNativeValidate (ngSubmit)="submit($event)" *ngIf="role"> <mat-card-content> - <mat-form-field *ngIf="!new"> - <mat-label>ID</mat-label> - <input matInput type="text" name="name" disabled readonly [defaultValue]="role.name" /> + <mat-form-field> + <mat-label>Name</mat-label> + <input matInput type="text" name="name" required [(ngModel)]="role.name" /> </mat-form-field> <mat-form-field> - <mat-label>ASN</mat-label> - <input matInput type="text" name="description" required [(ngModel)]="role.description" /> + <mat-label>Description</mat-label> + <input matInput type="text" name="description" [(ngModel)]="role.description" /> </mat-form-field> <mat-form-field> - <mat-label>Cache Group</mat-label> - <mat-select name="permissions" [(ngModel)]="role.permissions" required> - <mat-option *ngFor="" [value]="">{{}}</mat-option> - </mat-select> + <mat-label>Permissions</mat-label> + <textarea name="permissions" [(ngModel)]="role.permissions" matInput rows="10" wrap="hard" cdkTextareaAutosize></textarea> </mat-form-field> - <mat-form-field *ngIf="!new"> - <mat-label>Last Updated</mat-label> - <input matInput type="text" name="lastUpdated" disabled readonly [defaultValue]="role.lastUpdated" /> </mat-form-field> </mat-card-content> <mat-card-actions align="end"> <button mat-raised-button type="button" *ngIf="!new" color="warn" (click)="deleteRole()">Delete</button> diff --git a/experimental/traffic-portal/src/app/core/users/roles/detail/role-detail.component.spec.ts b/experimental/traffic-portal/src/app/core/users/roles/detail/role-detail.component.spec.ts index 7bbe9d896f..6e1d180395 100644 --- a/experimental/traffic-portal/src/app/core/users/roles/detail/role-detail.component.spec.ts +++ b/experimental/traffic-portal/src/app/core/users/roles/detail/role-detail.component.spec.ts @@ -12,11 +12,12 @@ * limitations under the License. */ import { ComponentFixture, TestBed } from "@angular/core/testing"; -import { MatDialogModule } from "@angular/material/dialog"; +import { MatDialog, MatDialogModule, type MatDialogRef } from "@angular/material/dialog"; import { ActivatedRoute } from "@angular/router"; import { RouterTestingModule } from "@angular/router/testing"; -import { ReplaySubject } from "rxjs"; +import { of, ReplaySubject } from "rxjs"; +import { UserService } from "src/app/api"; import { APITestingModule } from "src/app/api/testing"; import { RoleDetailComponent } from "src/app/core/users/roles/detail/role-detail.component"; import { NavigationService } from "src/app/shared/navigation/navigation.service"; @@ -57,21 +58,15 @@ describe("RoleDetailComponent", () => { fixture.detectChanges(); await fixture.whenStable(); expect(paramMap).toHaveBeenCalled(); - expect(component.roles).not.toBeNull(); - expect(component.roles.name).toBe(1); + expect(component.role).not.toBeNull(); + expect(component.role.name).toBe(""); expect(component.new).toBeTrue(); }); it("existing role", async () => { - paramMap.and.returnValue("1"); - - fixture = TestBed.createComponent(RoleDetailComponent); - component = fixture.componentInstance; - fixture.detectChanges(); - await fixture.whenStable(); expect(paramMap).toHaveBeenCalled(); - expect(component.roles).not.toBeNull(); - expect(component.roles.name).toBe(0); + expect(component.role).not.toBeNull(); + expect(component.role.name).toBe(""); expect(component.new).toBeFalse(); }); }); diff --git a/experimental/traffic-portal/src/app/core/users/roles/detail/role-detail.component.ts b/experimental/traffic-portal/src/app/core/users/roles/detail/role-detail.component.ts index 9e7897eee3..ba81b5170f 100644 --- a/experimental/traffic-portal/src/app/core/users/roles/detail/role-detail.component.ts +++ b/experimental/traffic-portal/src/app/core/users/roles/detail/role-detail.component.ts @@ -31,9 +31,10 @@ import { NavigationService } from "src/app/shared/navigation/navigation.service" }) export class RoleDetailComponent implements OnInit { public new = false; - public asn!: ResponseRole; - constructor(private readonly route: ActivatedRoute, private readonly location: Location, - private readonly dialog: MatDialog, private readonly header: NavigationService) { + public role!: ResponseRole; + constructor(private readonly route: ActivatedRoute, private readonly userService: UserService, + private readonly location: Location, private readonly dialog: MatDialog, + private readonly header: NavigationService) { } /** @@ -50,15 +51,14 @@ export class RoleDetailComponent implements OnInit { this.header.headerTitle.next("New Role"); this.new = true; this.role = { - description: "Read Only", - lastUpdated: new Date(), - name: "test", + description: "", + name: "", permissions: [] }; return; } - this.role = await this.UserService.getRoles(role); + this.role = await this.userService.getRoles(role); this.header.headerTitle.next(`Role: ${this.role.name}`); } @@ -76,7 +76,7 @@ export class RoleDetailComponent implements OnInit { }); ref.afterClosed().subscribe(result => { if(result) { - this.UserService.deleteRole(this.role.name); + this.userService.deleteRole(this.role); this.location.back(); } }); @@ -91,10 +91,10 @@ export class RoleDetailComponent implements OnInit { e.preventDefault(); e.stopPropagation(); if(this.new) { - this.asn = await this.UserService.createRole(this.role); + this.role = await this.userService.createRole(this.role); this.new = false; } else { - this.asn = await this.UserService.updateRoleN(this.Role); + this.role = await this.userService.updateRole(this.role); } } diff --git a/experimental/traffic-portal/src/app/core/users/roles/table/roles-table.component.spec.ts b/experimental/traffic-portal/src/app/core/users/roles/table/roles-table.component.spec.ts index 55d1ad2088..50455309ff 100644 --- a/experimental/traffic-portal/src/app/core/users/roles/table/roles-table.component.spec.ts +++ b/experimental/traffic-portal/src/app/core/users/roles/table/roles-table.component.spec.ts @@ -13,10 +13,12 @@ */ import { ComponentFixture, fakeAsync, TestBed, tick } from "@angular/core/testing"; -import { MatDialogModule } from "@angular/material/dialog"; +import { MatDialog, MatDialogModule, type MatDialogRef } from "@angular/material/dialog"; import { RouterTestingModule } from "@angular/router/testing"; import { ResponseRole } from "trafficops-types"; +import { of } from "rxjs"; +import { UserService } from "src/app/api"; import { APITestingModule } from "src/app/api/testing"; import { RolesTableComponent } from "src/app/core/users/roles/table/roles-table.component"; import { isAction } from "src/app/shared/generic-table/generic-table.component"; @@ -35,8 +37,7 @@ describe("RolesTableComponent", () => { await TestBed.configureTestingModule({ declarations: [ RolesTableComponent ], imports: [ APITestingModule, RouterTestingModule, MatDialogModule ] - }) - .compileComponents(); + }).compileComponents(); fixture = TestBed.createComponent(RolesTableComponent); component = fixture.componentInstance; @@ -93,18 +94,38 @@ describe("RolesTableComponent", () => { expect(item.href(role)).toBe(role.name); }); - it("has context menu items that aren't implemented yet", () => { - const item = component.contextMenuItems.find(i => i.name === "Edit"); + it("deletes Roles", fakeAsync(async () => { + const item = component.contextMenuItems.find(i => i.name === "Delete"); if (!item) { - return fail("missing 'Edit' context menu item"); + return fail("missing 'Delete' context menu item"); } - if (isAction(item)) { - return fail("incorrect type for 'Edit' menu item. Expected an action, not a link"); - } - if (typeof(item.disabled) !== "function") { - return fail("'Edit' context menu item should be disabled, but no disabled function is defined"); + if (!isAction(item)) { + return fail("incorrect type for 'Delete' menu item. Expected an action, not a link"); } - }); + expect(item.multiRow).toBeFalsy(); + expect(item.disabled).toBeUndefined(); + + const api = TestBed.inject(UserService); + const spy = spyOn(api, "deleteRole").and.callThrough(); + expect(spy).not.toHaveBeenCalled(); + + const dialogService = TestBed.inject(MatDialog); + const openSpy = spyOn(dialogService, "open").and.returnValue({ + afterClosed: () => of(true) + } as MatDialogRef<unknown>); + + const testRole = await api.createRole(role); + expect(openSpy).not.toHaveBeenCalled(); + const asyncExpectation = expectAsync(component.handleContextMenu({action: "delete", data: testRole})).toBeResolvedTo(undefined); + tick(); + + expect(openSpy).toHaveBeenCalled(); + tick(); + + expect(spy).toHaveBeenCalled(); + + await asyncExpectation; + })); it("generate 'View Users' context menu item href", () => { const item = component.contextMenuItems.find(i => i.name === "View Users"); diff --git a/experimental/traffic-portal/src/app/core/users/roles/table/roles-table.component.ts b/experimental/traffic-portal/src/app/core/users/roles/table/roles-table.component.ts index 6e65b7ed2e..5e00253cce 100644 --- a/experimental/traffic-portal/src/app/core/users/roles/table/roles-table.component.ts +++ b/experimental/traffic-portal/src/app/core/users/roles/table/roles-table.component.ts @@ -20,8 +20,10 @@ import type { ResponseRole } from "trafficops-types"; import { UserService } 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 } from "src/app/shared/generic-table/generic-table.component"; import { NavigationService } from "src/app/shared/navigation/navigation.service"; +import {MatDialog} from "@angular/material/dialog"; /** * RolesTableComponent is the controller for the "Roles" table. */ @@ -34,7 +36,7 @@ export class RolesTableComponent implements OnInit { /** List of roles */ public roles: Promise<Array<ResponseRole>>; constructor(private readonly route: ActivatedRoute, private readonly headerSvc: NavigationService, - private readonly api: UserService, public readonly auth: CurrentUserService) { + private readonly api: UserService, private readonly dialog: MatDialog, public readonly auth: CurrentUserService) { this.fuzzySubject = new BehaviorSubject<string>(""); this.roles = this.api.getRoles(); this.headerSvc.headerTitle.next("Roles"); @@ -77,7 +79,6 @@ export class RolesTableComponent implements OnInit { /** Definitions for the context menu items (which act on augmented roles data). */ public contextMenuItems: Array<ContextMenuItem<ResponseRole>> = [ { - disabled: (): true => true, href: (selectedRow: ResponseRole): string => `${selectedRow.name}`, name: "Edit" }, @@ -86,6 +87,11 @@ export class RolesTableComponent implements OnInit { name: "Open in New Tab", newTab: true }, + { + action: "delete", + multiRow: false, + name: "Delete" + }, { href: "/core/users", name: "View Users", @@ -107,9 +113,25 @@ export class RolesTableComponent implements OnInit { /** * Handles a context menu event. * - * @param a The action selected from the context menu. + * @param evt The action selected from the context menu. */ - public handleContextMenu(a: ContextMenuActionEvent<Readonly<ResponseRole>>): void { - console.log("action:", a); + public async handleContextMenu(evt: ContextMenuActionEvent<ResponseRole>): Promise<void> { + if (Array.isArray(evt.data)) { + console.error("cannot delete multiple roles at once:", evt.data); + return; + } + const data = evt.data; + switch(evt.action) { + case "delete": + const ref = this.dialog.open(DecisionDialogComponent, { + data: {message: `Are you sure you want to delete '${data.name}' role?`, title: "Confirm Delete"} + }); + ref.afterClosed().subscribe(result => { + if(result) { + this.api.deleteRole(data.name).then(async () => this.roles = this.api.getRoles()); + } + }); + break; + } } }
