Refactored Action Filters #261
Conversation
* ActionFilterAttribute is splitted into interfaces - request filter, viewModel filter and command filter * Now they also work when applied to IDotvvmPresenter and global request filters are applied to requests to any presenter not only the default one
| { | ||
| foreach (var f in filters) | ||
| { | ||
| await f.OnPageExceptionAsync(context, exception); |
There was a problem hiding this comment.
The method is now called also for DotvvmControlException and DotvvmHttpException rethrown in DotvvmPresenter (L66, L221). Is this intended?
There was a problem hiding this comment.
DotvvmPresenter rethrows only HttpException, DotvvmControlException should be handled like any other. I'll fix it.
| foreach (var f in filters) | ||
| { | ||
| await f.OnPageExceptionAsync(context, exception); | ||
| if (context.IsCommandExceptionHandled) context.InterruptRequest(); |
There was a problem hiding this comment.
Shouldn't we check IsPageExceptionHandled instead?
|
I like this PR, but we should probably consider more changes:
|
|
@djanosik I have just fixed the little issues you reported, thank you for review. |
|
@exyi I see, it'd be a breaking change. That's unfortunate, the name feels a bit confusing. Couldn't we keep |
|
@djanosik It is not a solution because it is an interface user will implement :(. And I actually don't think that the name is confusing, there is only better alternative. |
|
@exyi Ok then. :) |
…, supported on IDotvvmPresenter
|
@djanosik Authorize attribute should be working correctly on |
Review it please.