Add admin controllers for cities, polls and ideas#62
Add admin controllers for cities, polls and ideas#62SubochArtem wants to merge 23 commits intofeature/polls-cqrs-handlers-ideasfrom
Conversation
…hangeIdeaStatusCommand
…e filter in pollRepository
…hangePollStatusCommand
|
|
||
| public static class ClaimsPrincipalExtensions | ||
| { | ||
| private const string CityIdClaim = "https://citypulse.com/city_id"; |
There was a problem hiding this comment.
Why not store it in appsettings?
There was a problem hiding this comment.
I preferred a constant over appsettings in this situation because:
- This data is consistent across all environments and doesn't need to be configurable.
- This specific claim key is only used within this extension class, so it doesn't need global visibility.
ClaimsPrincipalExtensionsis a static class, which makes it difficult to use the Options pattern or IConfiguration
There was a problem hiding this comment.
Why using both result pattern and ExceptionMiddleware?
Every Internal Exception will be thrown without it
There was a problem hiding this comment.
I use Result pattern for expected domain logic. The ExceptionMiddleware is there because I don't want to leak internal error details to user. It allows me to control the output, and ensure every crash is logged in a standardized format
|
| var policy = new AuthorizationPolicyBuilder() | ||
| .RequireAuthenticatedUser() | ||
| .RequireClaim(Permissions.ClaimType, policyName) | ||
| .Build(); | ||
|
|
||
| return Task.FromResult<AuthorizationPolicy?>(policy); |
There was a problem hiding this comment.
There is no async work .Using async without await would create a state machine unnecessarily. Task.FromResult returns an already-completed task with zero overhead
4 participants
No description provided.