From e94a17f10d70f50f82648d552d553a2e8da6f689 Mon Sep 17 00:00:00 2001 From: nicdavidson Date: Thu, 15 Oct 2026 12:57:09 -0600 Subject: [PATCH] fix(roles): save works after removing a just-added access row A service-access row added and then removed before saving stayed in the form array with access set to '', so onSubmit threw 'access.reduce is not a function' and Update did nothing. Rows that were never saved are now left out of the payload, new rows start with access: [], and the row trash button is type=button so it cannot submit the form. e2e/role-save-after-remove.spec.ts covers adding-then-removing and removing a saved row. --- e2e/role-save-after-remove.spec.ts | 138 ++++++++++++++++++ .../df-role-details.component.ts | 12 +- .../df-roles-access.component.html | 2 +- .../df-roles-access.component.ts | 2 +- 4 files changed, 148 insertions(+), 6 deletions(-) create mode 100644 e2e/role-save-after-remove.spec.ts diff --git a/e2e/role-save-after-remove.spec.ts b/e2e/role-save-after-remove.spec.ts new file mode 100644 index 00000000..d977852f --- /dev/null +++ b/e2e/role-save-after-remove.spec.ts @@ -0,0 +1,138 @@ +import { test, expect, APIRequestContext, Page } from '@playwright/test'; +import { + ADMIN_EMAIL, + ADMIN_PASSWORD, + loginAsAdmin, + waitForAppReady, +} from './fixtures/admin-login'; + +/** + * Role access rows: removing a row and saving. A row added then removed + * before saving used to crash onSubmit (`access.reduce is not a function`), + * so Update did nothing. The trash button also submitted the form. + */ + +// Login + role load + save is ~30s against a cold instance. +test.describe.configure({ timeout: 90_000 }); + +const ROW = 'tr:has(mat-select[formcontrolname="service"])'; + +async function seedRole(request: APIRequestContext) { + const auth = await request.post('/api/v2/system/admin/session', { + data: { email: ADMIN_EMAIL, password: ADMIN_PASSWORD }, + }); + const headers = { + 'X-DreamFactory-Session-Token': (await auth.json()).session_token, + }; + const access = (component: string) => ({ + service_id: null, + component, + verb_mask: 1, + requestor_mask: 1, + }); + const res = await request.post('/api/v2/system/role', { + headers, + data: { + resource: [ + { + name: `e2e-role-remove-${Date.now()}`, + is_active: true, + role_service_access_by_role_id: [access('*'), access('_table/*')], + }, + ], + }, + }); + expect(res.ok(), await res.text()).toBe(true); + const id: number = (await res.json()).resource[0].id; + const accessRows = async () => + ( + await ( + await request.get( + `/api/v2/system/role/${id}?related=role_service_access_by_role_id`, + { headers } + ) + ).json() + ).role_service_access_by_role_id; + const cleanup = () => + request.delete(`/api/v2/system/role/${id}`, { headers }); + return { id, accessRows, cleanup }; +} + +async function openRole(page: Page, id: number) { + const pageErrors: string[] = []; + page.on('pageerror', e => pageErrors.push(e.message)); + const writes: string[] = []; + page.on('request', r => { + if (r.url().includes('/system/role') && r.method() !== 'GET') + writes.push(r.method()); + }); + await loginAsAdmin(page); + await waitForAppReady(page); + await page.goto( + `/dreamfactory/dist/#/api-connections/role-based-access/${id}` + ); + await expect(page.locator(ROW)).toHaveCount(2, { timeout: 15_000 }); + return { pageErrors, writes }; +} + +const trash = (page: Page) => + page.locator(ROW).last().locator('td:last-child button'); + +async function save(page: Page) { + const saved = page.waitForResponse( + r => + r.url().includes('/system/role') && + ['PUT', 'PATCH'].includes(r.request().method()), + { timeout: 10_000 } + ); + await page.getByRole('button', { name: /^(update|save)$/i }).click(); + expect((await saved).ok()).toBe(true); + await page.waitForURL(url => !/role-based-access\/\d+/.test(url.toString())); +} + +test('Roles: save after adding then removing an access row', async ({ + page, + request, +}) => { + const role = await seedRole(request); + try { + const { pageErrors, writes } = await openRole(page, role.id); + const rows = page.locator(ROW); + + await page + .getByRole('button', { name: /new entry/i }) + .first() + .click(); + await expect(rows).toHaveCount(3); + await trash(page).click(); + await expect(rows).toHaveCount(2); + expect(writes, 'removing a row must not save the role').toEqual([]); + + await save(page); + expect(pageErrors).toEqual([]); + expect(await role.accessRows()).toHaveLength(2); + } finally { + await role.cleanup(); + } +}); + +test('Roles: removing a saved access row deletes it on save', async ({ + page, + request, +}) => { + const role = await seedRole(request); + try { + const { pageErrors, writes } = await openRole(page, role.id); + const rows = page.locator(ROW); + + await trash(page).click(); + await expect(rows).toHaveCount(1); + expect(writes, 'removing a row must not save the role').toEqual([]); + + await save(page); + expect(pageErrors).toEqual([]); + expect(await role.accessRows()).toHaveLength(1); + } finally { + await role.cleanup(); + } +}); diff --git a/src/app/adf-roles/df-role-details/df-role-details.component.ts b/src/app/adf-roles/df-role-details/df-role-details.component.ts index 545e7073..d27b65ab 100644 --- a/src/app/adf-roles/df-role-details/df-role-details.component.ts +++ b/src/app/adf-roles/df-role-details/df-role-details.component.ts @@ -242,8 +242,12 @@ export class DfRoleDetailsComponent implements OnInit { name: formValue.name, description: formValue.description, isActive: formValue.active, - roleServiceAccessByRoleId: formValue.serviceAccess.map( - (val: AccessForm, index: number) => { + roleServiceAccessByRoleId: formValue.serviceAccess + .map((val: AccessForm, index: number) => { + // A row added and removed before saving never reached the + // server, so there is nothing to delete; its empty fields would + // also break the mask reduce below. + if (!this.visibilityArray[index] && !val.id) return null; const filtersArray = val.advancedFilters.map((filter: any) => ({ name: filter.expandField, operator: filter.expandOperator, @@ -263,8 +267,8 @@ export class DfRoleDetailsComponent implements OnInit { filters: filtersArray, filterOp: filterOp[0], }; - } - ), + }) + .filter(Boolean), lookupByRoleId: this.getLookupKeysWithDeletions(formValue), }; const createPayload = { diff --git a/src/app/adf-roles/df-roles-access/df-roles-access.component.html b/src/app/adf-roles/df-roles-access/df-roles-access.component.html index 79801113..942ce8c7 100644 --- a/src/app/adf-roles/df-roles-access/df-roles-access.component.html +++ b/src/app/adf-roles/df-roles-access/df-roles-access.component.html @@ -202,7 +202,7 @@ mat-cell *matCellDef="let row; let i = dataIndex" [formGroupName]="getFormArrayIndex(i)"> - diff --git a/src/app/adf-roles/df-roles-access/df-roles-access.component.ts b/src/app/adf-roles/df-roles-access/df-roles-access.component.ts index d94131cf..64174308 100644 --- a/src/app/adf-roles/df-roles-access/df-roles-access.component.ts +++ b/src/app/adf-roles/df-roles-access/df-roles-access.component.ts @@ -302,7 +302,7 @@ export class DfRolesAccessComponent implements OnInit { new FormGroup({ service: new FormControl(0, Validators.required), component: new FormControl('', Validators.required), - access: new FormControl('', Validators.required), + access: new FormControl([], Validators.required), requester: new FormControl([1], Validators.required), advancedFilters: advancedFilters, id: new FormControl(null),