-
Notifications
You must be signed in to change notification settings - Fork 114
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
Updated FeatureGateAttribute to be able to be used on Razor Pages. #166
Updated FeatureGateAttribute to be able to be used on Razor Pages. #166
Conversation
…filters and action filter execution is isolated.
<ItemGroup> | ||
<None Remove="appsettings.json" /> | ||
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<Content Include="appsettings.json"> | ||
<CopyToOutputDirectory>Always</CopyToOutputDirectory> | ||
<CopyToPublishDirectory>PreserveNewest</CopyToPublishDirectory> | ||
</Content> | ||
</ItemGroup> |
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.
Curious to know why we don't need to override these options anymore.
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.
In order to use Razor Pages in the test project I had to update the sdk type to Microsoft.NET.Sdk.Web
. The default settings for that sdk type suffice.
|
||
<PropertyGroup> | ||
<TargetFrameworks>netcoreapp2.1;netcoreapp3.1;net5.0</TargetFrameworks> | ||
<TargetFrameworks>netcoreapp3.1;net5.0</TargetFrameworks> |
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.
While we are at it, should we also add .NET6 to the targets since it is the latest LTS release of .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.
Yeah, it's a good idea. but that would be better in a separate PR to add .NET 6 targeting.
src/Microsoft.FeatureManagement.AspNetCore/Microsoft.FeatureManagement.AspNetCore.csproj
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.
Looks good to me!
src/Microsoft.FeatureManagement.AspNetCore/FeatureGateAttribute.cs
Outdated
Show resolved
Hide resolved
} | ||
else | ||
{ | ||
context.Result = new NotFoundResult(); |
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.
Should we allow users to customize the behavior like for actions? #Closed
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.
It's hard to say, I left it out to see if it is a requested feature. In the issue most proposals seemed to be fine with 404 responses on pages with disabled features. I have no problem with adding it.
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.
Makes sense. We can always add it in the future.
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.
* Updated FeatureGateAttribute to be able to be used on Razor Pages. (#166) * Add PageFeatureGate * Fix tests. * Remove test page. * Added missing license headers. * Bundle PageFeatureGateAttribute functionality into FeatureGate. Page filters and action filter execution is isolated. * Use OkResult instead of StatusCode. Updated indenting. * Remove unnecessary virtual for interface implementation. * Add back netcoreapp2.1 in tests. * Remove tabs from csproj * Update package versions. (#167) * Update type/method summaries in FeatureGateAttribute. (#170) * Update type/method summaries in FeatureGateAttribute. * Add back text. * Add documentation for FeatureGateAttribute usage on Razor pages. (#169) * Microsoft mandatory file (#177) Co-authored-by: microsoft-github-policy-service[bot] <77245923+microsoft-github-policy-service[bot]@users.noreply.github.com> Co-authored-by: microsoft-github-policy-service[bot] <77245923+microsoft-github-policy-service[bot]@users.noreply.github.com>
See #34 for discussion leading to this PR.
Currently the library supports controlling visibility of controllers with the
FeatureGateAttribute
. This works with MVC controllers and actions. It does not work with Razor pages. This is because theFeatureGateAttribute
only runs as an MVC action filter. Code execution in the Razor Page pipeline is performed via page handlers.By updating the
FeatureGateAttribute
to implementIAsyncPageFilter
we are capable of supporting Razor Pages. MVC controllers only execute the action filter portion ofFeatureGateAttribute
, while Razor Pages only execute the page filter portion.Usage