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

Add context to validation function #146

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

paolosanchi
Copy link

@paolosanchi paolosanchi commented Aug 13, 2020

As required by #145 I've added the ability to pass a "context" object/any to the validation function, so it could return true or false based on the context where is used.

I've choose to put the context object as the third parameter because of retro-compatibility reasons:

export declare type ValidationFn =
    ((name?: string, store?: any, context?: any) => Promise<void | string | boolean> | boolean | string[]);

The resulting usage is this (in a real project, where the validation depends on the state of a report entity):
Directive:

        <ng-container *ngxPermissionsOnly="'reportEdit';context:report">
          <ion-button fill="clear" size="small" (click)="editReport()">
            <ion-icon name="document"></ion-icon>
            <div>Edit Report</div>
          </ion-button>
        </ng-container>

or

        <ng-template [ngxPermissionsOnly]="'reportEdit'" [ngxPermissionsOnlyContext]="report">
          <ion-button fill="clear" size="small" (click)="editReport()">
            <ion-icon name="document"></ion-icon>
            <div>Edit Report</div>
          </ion-button>
        </ng-template>

Permission declaration:

        this.permissionsService.addPermission('reportEdit', (name?: string, store?: any, report?: any) => {
            if (report) {
                return ((report.state == ReportState.Draft || report.state == ReportState.Sent) && !report.seen);
            }
        });

Service hasPermission():

this.permissionsService.hasPermission('reportEdit', report);

Routing
Here comes an issue, we need the report to do the permission validation, the right way would be to "resove" and have it available in the controller, too.
The problem is that the resolvers are evaluated after the guards (https://stackoverflow.com/questions/39190427/angular2-resolve-before-canactivate).
The only solution I can figure out is to create a custom validator that fetch the report, whom is fetched a second time in the controller (or resolver).
This is an example (not fully tested):

this.permissionsService.addPermission('reportEditById', async (name?: string, store?: any, reportId?: any) => {
	const report = awit this.reportService.get(reportId);
	if (report) {
		return ((report.state == ReportState.Draft || report.state == ReportState.Sent) && !report.seen);
	} 
});

let routes = [
  { path: '', 
    canActivate: [AuthGuard],
    children: [
      {
	  path: 'edit', 
          component: ReportEditComponent, 
          canActivate: [NgxPermissionsGuard],
          data: {
          permissions: {
           only: 'reportEditById',
           context:  (route: ActivatedRouteSnapshot, state: RouterStateSnapshot) => {
            const reportId = route.paramMap.get('id');           
            return reportId;
           },
           redirectTo: 'home'
         }
       }}
      ]
  }
]

Note:
I've added all the tests for the service, but omitted the ones for the directive and the router, I've tried to add some locally and it seems to work, the issue is that really I don't know what and how to test, and I feel I don't really know what I'm doing. I'd like to test what happen if the context changes, but I'm not confident with my testing skills.

The docs has not been edited as well, my english is not really good ;)

@AlexKhymenko
Copy link
Owner

@paolosanchi Hi. Thank You very much. I will review as soon as have time. Thanks very much!!

@rafaelkachlon
Copy link

Hey @AlexKhymenko , any news with the PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants