-
Notifications
You must be signed in to change notification settings - Fork 21
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
feat(permissions): filter fields in forms based on read/write permissions #2180
Conversation
…ions - filter fields in forms with no read permission - disable fields in edit mode with read-only permissions
Deployed to https://pr-2180.aam-digital.net/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the goods start already. The functionality looks good, lets try to refactor this so we can use the same approach in all forms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am currently wondering whether instead of listening to the status changes, we can also just overwrite the enable
function of the FormGroup
. So a subclass FormGroupWithPermission
so this whole "what can be enabled" would all be built-in instead of managed from the outside. For me this feels a bit more straight forward, how well the general extending of Angular classes works in the long run, I am not sure. Lets discuss this.
src/app/core/common-components/entity-form/entity-form.service.spec.ts
Outdated
Show resolved
Hide resolved
src/app/core/common-components/entity-form/entity-form/entity-form.component.ts
Outdated
Show resolved
Hide resolved
src/app/core/common-components/entity-form/entity-form.service.spec.ts
Outdated
Show resolved
Hide resolved
src/app/core/common-components/entity-form/entity-form.service.spec.ts
Outdated
Show resolved
Hide resolved
src/app/core/common-components/entity-form/entity-form.service.ts
Outdated
Show resolved
Hide resolved
src/app/core/common-components/entity-form/entity-form/entity-form.component.ts
Outdated
Show resolved
Hide resolved
src/app/core/common-components/entity-select/entity-select.component.spec.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Simon <simon@aam-digital.com>
Co-authored-by: Simon <simon@aam-digital.com>
…read-or-write-perissions
src/app/core/common-components/entity-form/entity-form/entity-form.component.ts
Outdated
Show resolved
Hide resolved
src/app/core/common-components/entity-form/entity-form.service.spec.ts
Outdated
Show resolved
Hide resolved
src/app/core/common-components/entity-form/entity-form/entity-form.component.spec.ts
Outdated
Show resolved
Hide resolved
src/app/core/common-components/entity-form/entity-form.service.ts
Outdated
Show resolved
Hide resolved
src/app/core/common-components/entity-form/entity-form/entity-form.component.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks very good now. I only changed some small things. When playing with it I discovered a functionality issue though:
Currently the disabled
state is always applied based on the update
permissions. This means that the property is also disabled when a new entity (e.g. a new Note) is created even if the user has create
permissions on the property. I think there is a valid use case where users are allowed to set a property initially, but once the entity already exists they are not allowed to change it anymore. Therefore I think for new entities the disabled state should be evaluated based on the create
permission for a field.
src/app/core/common-components/entity-form/entity-form.service.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works great now. Thanks.
…read-or-write-perissions
🎉 This PR is included in version 3.30.0-master.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
🎉 This PR is included in version 3.30.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Can be tested with this permissions:
related to: #1912
Visible/Frontend Changes
Architectural/Backend Changes