#3 Show certain fields based on permissions#31
Conversation
ishehata
left a comment
ishehata
left a comment
There was a problem hiding this comment.
I believe the permissions should be part of the FieldConfig attributes or Metadata, so that no naming collisions would happen.
The permissions service check method would take metadata or FieldConfig and check user permissions against it.
|
@ishehata |
|
@ishehata |
ishehata
left a comment
ishehata
left a comment
There was a problem hiding this comment.
I am thinking about the permissions attributes, write now they are create, read ... etc,
what if they were dynamic, and the developer would be able to add they keys that suitable for his use case, need to think about it though, and how to do implement it. @thelinuxer what do you think?
| <h4> {{ config.label }}</h4> | ||
| <span class="spacer"></span> | ||
| <button mat-icon-button (click)="addForm()"> | ||
| <button mat-icon-button (click)="addForm()" *ngIf="permissionsService.checkPermission(config?.permissions, permissionTypeEnum.update)"> |
There was a problem hiding this comment.
Can you convert the permissionsService to private in the container, and create a function in the component called checkPermissions that makes use of permissionsService ? it would be cleaner this way, and I was thinking maybe the permissions service in the future would return observables instead.
| @@ -1,8 +1,10 @@ | |||
| import { PermissionType } from './../../models/permissions'; | |||
There was a problem hiding this comment.
can you please move this to line 8, just to maintain order of imports, and keep them grouped ?
I think we should accept the current behaviour and have a change request for the permission modifications. That's of course if you think the default behaviour is usable enough. |
|
@ishehata are we done with this pull request ? Can it be merged ? |
|
Still waiting for minor changes |
3 participants
No description provided.