From 33f9adb05acccde338a200a6031b9fac2f500a2a Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Sun, 23 Aug 2026 19:42:18 -0700 Subject: [PATCH] Chore: refactor permission checkbox live changes (#13771) --- .../permissions-select.component.spec.ts | 19 ++++ .../permissions-select.component.ts | 96 ++++++++++++------- 2 files changed, 80 insertions(+), 35 deletions(-) diff --git a/src-ui/src/app/components/common/permissions-select/permissions-select.component.spec.ts b/src-ui/src/app/components/common/permissions-select/permissions-select.component.spec.ts index dd82cdac5..dd932843b 100644 --- a/src-ui/src/app/components/common/permissions-select/permissions-select.component.spec.ts +++ b/src-ui/src/app/components/common/permissions-select/permissions-select.component.spec.ts @@ -107,6 +107,25 @@ describe('PermissionsSelectComponent', () => { expect(component.form.get('Tag').get('Change').disabled).toBeTruthy() }) + it('should update checkboxes when inherited permissions change', () => { + component.ngOnInit() + component.inheritedPermissions = ['documents.change_document'] + component.writeValue(['delete_document']) + expect(component.form.get('Document').get('Change').value).toBeTruthy() + expect(component.form.get('Document').get('Change').disabled).toBeTruthy() + + // swap for a group with a different permission, but the same number of them + component.inheritedPermissions = ['documents.view_document'] + + // the no-longer-inherited permission is unchecked, the explicit one is kept + expect(component.permissions).toEqual(['delete_document']) + expect(component.form.get('Document').get('Change').value).toBeFalsy() + expect(component.form.get('Document').get('Change').disabled).toBeFalsy() + expect(component.form.get('Document').get('Delete').value).toBeTruthy() + expect(component.form.get('Document').get('View').value).toBeTruthy() + expect(component.form.get('Document').get('View').disabled).toBeTruthy() + }) + it('should exclude history permissions if disabled', () => { settingsService.set(SETTINGS_KEYS.AUDITLOG_ENABLED, false) fixture = TestBed.createComponent(PermissionsSelectComponent) diff --git a/src-ui/src/app/components/common/permissions-select/permissions-select.component.ts b/src-ui/src/app/components/common/permissions-select/permissions-select.component.ts index 2032e84e0..52030ac2c 100644 --- a/src-ui/src/app/components/common/permissions-select/permissions-select.component.ts +++ b/src-ui/src/app/components/common/permissions-select/permissions-select.component.ts @@ -74,12 +74,22 @@ export class PermissionsSelectComponent ? inherited.map((p) => p.replace(/^\w+\./g, '')) : [] - if (this._inheritedPermissions !== newInheritedPermissions) { - this._inheritedPermissions = newInheritedPermissions - this.writeValue(this.permissions) // updates visual checks etc. - } + const changed = + newInheritedPermissions.length !== this._inheritedPermissions.length || + newInheritedPermissions.some( + (p) => !this._inheritedPermissions.includes(p) + ) - this.updateDisabledStates() + if (changed) { + // skip inherited permissions, these are the explicitly set ones + this.permissions = this.getSelectedPermissions( + this.form.getRawValue() + ).filter((p) => !this._inheritedPermissions.includes(p)) + this._inheritedPermissions = newInheritedPermissions + this.applyCheckedState() + } else { + this.updateDisabledStates() + } } inheritedWarning: string = $localize`Inherited from group` @@ -106,20 +116,29 @@ export class PermissionsSelectComponent } this.permissions = permissions ?? [] - const allPerms = this._inheritedPermissions.concat(this.permissions) + this.applyCheckedState() + } - allPerms.forEach((permissionStr) => { - const { actionKey, typeKey } = - this.permissionsService.getPermissionKeys(permissionStr) + // sets every checkbox from inherited + own perms + private applyCheckedState(): void { + const allPerms = new Set( + this._inheritedPermissions.concat(this.permissions) + ) - if (actionKey && typeKey) { - this.form - .get(typeKey) - ?.get(actionKey) - ?.patchValue(true, { emitEvent: false }) - } - }) this.allowedTypes.forEach((type) => { + const typeGroup = this.form.get(type) + for (const action of Object.keys(PermissionAction)) { + typeGroup.get(action)?.patchValue( + allPerms.has( + this.permissionsService.getPermissionCode( + PermissionAction[action], + PermissionType[type] + ) + ), + { emitEvent: false } // don't trigger valueChanges now + ) + } + if (this.typeHasAllActionsSelected(type)) { this.typesWithAllActions.add(type) } else { @@ -150,26 +169,9 @@ export class PermissionsSelectComponent ngOnInit(): void { this.form.valueChanges.subscribe((newValue) => { - let permissions = [] - Object.entries(newValue).forEach(([typeKey, typeValue]) => { - const selectedActions = Object.entries(typeValue).filter( - ([actionKey, actionValue]) => - actionValue && - this.isActionSupported( - PermissionType[typeKey], - PermissionAction[actionKey] - ) - ) - - selectedActions.forEach(([actionKey]) => { - permissions.push( - (PermissionType[typeKey] as string).replace( - '%s', - PermissionAction[actionKey] - ) - ) - }) + const permissions = this.getSelectedPermissions(newValue) + Object.keys(newValue).forEach((typeKey) => { if (this.typeHasAllActionsSelected(typeKey)) { this.typesWithAllActions.add(typeKey) } else { @@ -269,6 +271,30 @@ export class PermissionsSelectComponent return true } + private getSelectedPermissions(formValue: object): string[] { + const permissions = [] + Object.entries(formValue).forEach(([typeKey, typeValue]) => { + Object.entries(typeValue) + .filter( + ([actionKey, actionValue]) => + actionValue && + this.isActionSupported( + PermissionType[typeKey], + PermissionAction[actionKey] + ) + ) + .forEach(([actionKey]) => { + permissions.push( + this.permissionsService.getPermissionCode( + PermissionAction[actionKey], + PermissionType[typeKey] + ) + ) + }) + }) + return permissions + } + private typeHasAllActionsSelected(typeKey: string): boolean { return Object.keys(PermissionAction) .filter((action) =>