Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(permissions): filter fields in forms based on read/write permissions #2180

Merged
Changes from 9 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
420e45f
feat(permissions): filter fields in forms based on read/write permiss…
tomwwinter Jan 15, 2024
0e627bc
fix: reset demo-permission-generator
tomwwinter Jan 15, 2024
ab8a9d6
refactor: disable-wrapper component for better Cognitive Complexity s…
tomwwinter Jan 15, 2024
efa5e04
Merge branch 'master' into tw/feat/1912-filter-fields-if-user-has-no-…
tomwwinter Jan 15, 2024
020d3bb
fix: use correct import path in ability.service.ts
tomwwinter Jan 15, 2024
e5301eb
refactor: move filter logic to entity-form component/service
tomwwinter Jan 15, 2024
38f8c81
fix: typo
tomwwinter Jan 15, 2024
41d779c
test: improve test coverage
tomwwinter Jan 15, 2024
ed6181e
Merge branch 'master' into tw/feat/1912-filter-fields-if-user-has-no-…
tomwwinter Jan 15, 2024
5248fe0
fix: test syntax
tomwwinter Jan 16, 2024
f99b8d0
fix: test syntax
tomwwinter Jan 16, 2024
754bf3e
fix: pr review feedback
tomwwinter Jan 16, 2024
cf79872
fix: typo
tomwwinter Jan 16, 2024
b1feafc
fix: typo
tomwwinter Jan 16, 2024
c2290c8
fix: pr review feedback
tomwwinter Jan 16, 2024
6e5a4f2
Merge branch 'master' into tw/feat/1912-filter-fields-if-user-has-no-…
tomwwinter Jan 18, 2024
b692321
fix: remove unnecessary function call
tomwwinter Jan 18, 2024
8142222
fix: code format
tomwwinter Jan 18, 2024
dd2f5ff
fix: code format
tomwwinter Jan 18, 2024
4988f07
Merge branch 'master' into tw/feat/1912-filter-fields-if-user-has-no-…
TheSlimvReal Jan 22, 2024
baecf55
disabled check also happens on form initialization
TheSlimvReal Jan 22, 2024
14e58a9
Merge branch 'master' into tw/feat/1912-filter-fields-if-user-has-no-…
tomwwinter Jan 24, 2024
429428d
fix: use create permissions when form creates new entity
tomwwinter Jan 24, 2024
28d6bdb
fix: use create permissions when form creates new entity
tomwwinter Jan 24, 2024
587ff54
removed unnecessary variable
TheSlimvReal Jan 25, 2024
017a45b
Merge branch 'master' into tw/feat/1912-filter-fields-if-user-has-no-…
TheSlimvReal Jan 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/app/app.component.spec.ts
Original file line number Diff line number Diff line change
@@ -41,11 +41,11 @@ describe("AppComponent", () => {
fixture.detectChanges();
}));

afterEach(() => {
afterEach(waitForAsync(() => {
environment.demo_mode = false;
jasmine.DEFAULT_TIMEOUT_INTERVAL = intervalBefore;
return TestBed.inject(Database).destroy();
});
}));

it("should be created", () => {
expect(component).toBeTruthy();
Original file line number Diff line number Diff line change
@@ -26,6 +26,7 @@ import { DatabaseField } from "../../entity/database-field.decorator";
import { EntitySchemaService } from "../../entity/schema/entity-schema.service";
import { FormFieldConfig } from "./FormConfig";
import { TEST_USER } from "../../user/demo-user-generator.service";
import { FieldGroup } from "../../entity-details/form/field-group";

describe("EntityFormService", () => {
let service: EntityFormService;
@@ -54,6 +55,46 @@ describe("EntityFormService", () => {
expect(entity.getId()).not.toBe("newId");
});

it("should remove fields without read permissions", async () => {
const entity = new Entity();
const fieldGroup: FieldGroup[] = [
{ fields: ["foo", "bar"] },
{ fields: ["name"] },
{ fields: ["birthday"] },
];

TestBed.inject(EntityAbility).update([
{ subject: "Entity", action: "read", fields: ["foo", "name"] },
]);

const filteredFieldGroup = service.filterFieldGroupsByPermissions(
fieldGroup,
entity,
);

expect(filteredFieldGroup.length).toBe(2);
expect(filteredFieldGroup[0].fields.length).toBe(1);
expect(filteredFieldGroup[1].fields.length).toBe(1);
tomwwinter marked this conversation as resolved.
Show resolved Hide resolved
});

it("should remove controls with read-only permissions from form", async () => {
const entity = new Entity();
const formGroup = new UntypedFormGroup({
name: new UntypedFormControl("name"),
foo: new UntypedFormControl("foo"),
bar: new UntypedFormControl("bar"),
});
TestBed.inject(EntityAbility).update([
{ subject: "Entity", action: "update", fields: ["foo"] },
]);

service.disableReadOnlyFormControls(formGroup, entity);
TheSlimvReal marked this conversation as resolved.
Show resolved Hide resolved

expect(formGroup.get("name").disabled).toBeTruthy();
expect(formGroup.get("foo").disabled).toBeFalse();
expect(formGroup.get("bar").disabled).toBeTruthy();
tomwwinter marked this conversation as resolved.
Show resolved Hide resolved
});
TheSlimvReal marked this conversation as resolved.
Show resolved Hide resolved

it("should update entity if saving is successful", async () => {
const entity = new Entity("initialId");
const formGroup = new UntypedFormGroup({
Original file line number Diff line number Diff line change
@@ -18,6 +18,7 @@ import {
} from "../../entity/schema/entity-schema-field";
import { isArrayDataType } from "../../basic-datatypes/datatype-utils";
import { CurrentUserSubject } from "../../session/current-user-subject";
import { FieldGroup } from "../../entity-details/form/field-group";

/**
* These are utility types that allow to define the type of `FormGroup` the way it is returned by `EntityFormService.create`
@@ -78,6 +79,35 @@ export class EntityFormService {
}
}

filterFieldGroupsByPermissions<T extends Entity = Entity>(
fieldGroups: FieldGroup[],
tomwwinter marked this conversation as resolved.
Show resolved Hide resolved
entity: T,
): FieldGroup[] {
return fieldGroups
.map((group) => {
group.fields = group.fields.filter((field) =>
this.ability.can(
"read",
entity,
typeof field === "string" ? field : field.id,
),
);
return group;
})
.filter((group) => group.fields.length > 0);
}

disableReadOnlyFormControls<T extends Entity>(
TheSlimvReal marked this conversation as resolved.
Show resolved Hide resolved
form: EntityForm<T>,
entity: T,
) {
Object.keys(form.controls).forEach((fieldId) => {
if (!this.ability.can("update", entity, fieldId)) {
form.get(fieldId).disable({ onlySelf: true, emitEvent: false });
}
});
}

private addSchemaToFormField(
formField: FormFieldConfig,
propertySchema: EntitySchemaField,
@@ -229,7 +259,7 @@ export class EntityFormService {
}

private checkFormValidity<T extends Entity>(form: EntityForm<T>) {
// errors regarding invalid fields wont be displayed unless marked as touched
// errors regarding invalid fields won't be displayed unless marked as touched
form.markAllAsTouched();
if (form.invalid) {
throw new InvalidFormFieldError();
Original file line number Diff line number Diff line change
@@ -6,7 +6,7 @@ import {
ViewEncapsulation,
} from "@angular/core";
import { Entity } from "../../../entity/model/entity";
import { EntityForm } from "../entity-form.service";
import { EntityForm, EntityFormService } from "../entity-form.service";
import { EntityMapperService } from "../../../entity/entity-mapper/entity-mapper.service";
import { filter } from "rxjs/operators";
import { ConfirmationDialogService } from "../../confirmation-dialog/confirmation-dialog.service";
@@ -60,9 +60,12 @@ export class EntityFormComponent<T extends Entity = Entity>
constructor(
private entityMapper: EntityMapperService,
private confirmationDialog: ConfirmationDialogService,
private entityFormService: EntityFormService,
) {}

ngOnChanges(changes: SimpleChanges) {
this.applyFormFieldPermissions();

if (changes.entity && this.entity) {
this.changesSubscription?.unsubscribe();
this.changesSubscription = this.entityMapper
@@ -78,6 +81,31 @@ export class EntityFormComponent<T extends Entity = Entity>
this.initialFormValues = this.form.getRawValue();
this.disableForLockedEntity();
}

if (this.form && this.entity) {
this.entityFormService.disableReadOnlyFormControls(
TheSlimvReal marked this conversation as resolved.
Show resolved Hide resolved
this.form,
this.entity,
);
}
}

private applyFormFieldPermissions() {
TheSlimvReal marked this conversation as resolved.
Show resolved Hide resolved
if (this.fieldGroups) {
this.fieldGroups = this.entityFormService.filterFieldGroupsByPermissions(
this.fieldGroups,
this.entity,
);
}

if (this.form) {
this.form.statusChanges.pipe(untilDestroyed(this)).subscribe((_) => {
TheSlimvReal marked this conversation as resolved.
Show resolved Hide resolved
this.entityFormService.disableReadOnlyFormControls(
this.form,
this.entity,
);
});
}
TheSlimvReal marked this conversation as resolved.
Show resolved Hide resolved
}

private async applyChanges(externallyUpdatedEntity: T) {
Original file line number Diff line number Diff line change
@@ -12,10 +12,12 @@ import { Child } from "../../../child-dev-project/children/model/child";
import { School } from "../../../child-dev-project/schools/model/school";
import { MockedTestingModule } from "../../../utils/mocked-testing.module";
import { LoginState } from "../../session/session-states/login-state.enum";
import { EntityAbility } from "../../permissions/ability/entity-ability";

describe("EntitySelectComponent", () => {
let component: EntitySelectComponent<any>;
let fixture: ComponentFixture<EntitySelectComponent<any>>;
let entityAbility: EntityAbility;

const testUsers: Entity[] = ["Abc", "Bcd", "Abd", "Aba"].map((s) => {
const user = new User();
@@ -36,6 +38,8 @@ describe("EntitySelectComponent", () => {
]),
],
}).compileComponents();
entityAbility = TestBed.inject(EntityAbility);
spyOn(entityAbility, "can").and.returnValue(true);
TheSlimvReal marked this conversation as resolved.
Show resolved Hide resolved
}));

beforeEach(() => {
5 changes: 5 additions & 0 deletions src/app/core/entity-details/form/form.component.spec.ts
Original file line number Diff line number Diff line change
@@ -7,16 +7,21 @@ import { MockedTestingModule } from "../../../utils/mocked-testing.module";
import { ConfirmationDialogService } from "../../common-components/confirmation-dialog/confirmation-dialog.service";
import { AlertService } from "../../alerts/alert.service";
import { EntityFormService } from "../../common-components/entity-form/entity-form.service";
import { EntityAbility } from "../../permissions/ability/entity-ability";

describe("FormComponent", () => {
let component: FormComponent<Child>;
let fixture: ComponentFixture<FormComponent<Child>>;

let entityAbility: EntityAbility;

beforeEach(waitForAsync(() => {
TestBed.configureTestingModule({
imports: [FormComponent, MockedTestingModule.withState()],
providers: [{ provide: ConfirmationDialogService, useValue: null }],
}).compileComponents();
entityAbility = TestBed.inject(EntityAbility);
spyOn(entityAbility, "can").and.returnValue(true);
}));

beforeEach(() => {
1 change: 1 addition & 0 deletions src/app/core/entity-details/form/form.component.ts
Original file line number Diff line number Diff line change
@@ -52,6 +52,7 @@ export class FormComponent<E extends Entity> implements FormConfig, OnInit {
[].concat(...this.fieldGroups.map((group) => group.fields)),
this.entity,
);

if (!this.creatingNew) {
this.form.disable();
}
Original file line number Diff line number Diff line change
@@ -21,11 +21,13 @@ import { TestbedHarnessEnvironment } from "@angular/cdk/testing/testbed";
import { MatTabGroupHarness } from "@angular/material/tabs/testing";
import { FormDialogService } from "../../form-dialog/form-dialog.service";
import { UpdatedEntity } from "../../entity/model/entity-update";
import { EntityAbility } from "../../permissions/ability/entity-ability";

describe("EntityListComponent", () => {
let component: EntityListComponent<Entity>;
let fixture: ComponentFixture<EntityListComponent<Entity>>;
let loader: HarnessLoader;
let entityAbility: EntityAbility;

const testConfig: EntityListConfig = {
title: "Children List",
@@ -94,6 +96,8 @@ describe("EntityListComponent", () => {
{ provide: FormDialogService, useValue: null },
],
}).compileComponents();
entityAbility = TestBed.inject(EntityAbility);
spyOn(entityAbility, "can").and.returnValue(true);
}));

it("should create", () => {
Original file line number Diff line number Diff line change
@@ -26,12 +26,13 @@ export class DisableEntityOperationDirective
{
/**
* These arguments are required to check whether the user has permissions to perform the operation.
* The operation property defines to what kind of operation a element belongs, e.g. OperationType.CREATE
* The operation property defines to what kind of operation an element belongs, e.g. OperationType.CREATE
* The entity property defines for which kind of entity the operation will be performed, e.g. Child
*/
@Input("appDisabledEntityOperation") arguments: {
operation: EntityAction;
entity: EntitySubject;
field?: string;
};

private wrapperComponent: ComponentRef<DisabledWrapperComponent>;
@@ -76,6 +77,7 @@ export class DisableEntityOperationDirective
this.wrapperComponent.instance.elementDisabled = this.ability.cannot(
this.arguments.operation,
this.arguments.entity,
this.arguments.field,
);
this.wrapperComponent.instance.ngAfterViewInit();
}
Original file line number Diff line number Diff line change
@@ -51,16 +51,31 @@ export class DisabledWrapperComponent implements AfterViewInit {
constructor(private renderer: Renderer2) {}

ngAfterViewInit() {
if (this.wrapper) {
const buttonElement =
this.wrapper.nativeElement.getElementsByTagName("button")[0];
if (this.elementDisabled) {
this.renderer.addClass(buttonElement, "mat-button-disabled");
this.renderer.setAttribute(buttonElement, "disabled", "true");
} else {
this.renderer.removeAttribute(buttonElement, "disabled");
this.renderer.removeClass(buttonElement, "mat-button-disabled");
}
if (!this.wrapper) {
return;
}

const buttonElement =
this.wrapper.nativeElement.getElementsByTagName("button")[0];

if (!buttonElement) {
return;
}

if (this.elementDisabled) {
this.disable(buttonElement);
} else {
this.enable(buttonElement);
}
}

private enable(buttonElement: HTMLButtonElement) {
this.renderer.removeAttribute(buttonElement, "disabled");
this.renderer.removeClass(buttonElement, "mat-button-disabled");
}

private disable(buttonElement: HTMLButtonElement) {
this.renderer.addClass(buttonElement, "mat-button-disabled");
this.renderer.setAttribute(buttonElement, "disabled", "true");
}
}