Skip to content
This repository was archived by the owner on Dec 14, 2018. It is now read-only.

Add convenience extension methods for IPageApplicationModelConvention#5866

Closed
pranavkm wants to merge 1 commit into
devfrom
prkrishn/PageConventionConvenienceMethods
Closed

Add convenience extension methods for IPageApplicationModelConvention#5866
pranavkm wants to merge 1 commit into
devfrom
prkrishn/PageConventionConvenienceMethods

Conversation

@pranavkm

Copy link
Copy Markdown
Contributor

No description provided.

@@ -0,0 +1,23 @@
using System;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copyright

@jbagga jbagga left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

services.TryAddSingleton<IActionDescriptorChangeProvider, PageActionDescriptorChangeProvider>();

services.TryAddSingleton<TempDataPropertyProvider>();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove

@NTaylorMullen NTaylorMullen left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super small comments. If null check question results in the need for null checks you can just add those.

// Internal for testing.
internal static void AddServices(IServiceCollection services)
{
services.TryAddEnumerable(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super hard to read with the lines broken up. Just put this entire bit on one line.

/// <param name="path">The path of the Razor Page.</param>
/// <param name="policy">The authorization policy.</param>
/// <returns>The <see cref="RazorPagesOptions"/>.</returns>
public static RazorPagesOptions AuthorizePage(this RazorPagesOptions options, string path, string policy)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is policy allowed to be null? Same q for other extensions that take policy.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neither the filter that we create here, nor the AuthorizeAttribute perform a null check for it. I think it's designed to be null.

@pranavkm pranavkm closed this Mar 6, 2017
@pranavkm
pranavkm deleted the prkrishn/PageConventionConvenienceMethods branch March 6, 2017 23:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants