Feature/interceptor for activities - Githubissues

ITsvetkoFF / Kv-013

7 stars 1 forks source link

Feature/interceptor for activities #152

Closed Gelphy closed 8 years ago

Gelphy commented 8 years ago

Add first interceptor for roles and delete unneeded tests and controllers Fix assign role attribute Add extensions and constants, refactor attribute Add extensions, refactor attribute Remove GetCollaborators method from extensions

[ ] File: DevelopGitHub/GitHubExtension/GitHubExtension.Activity.Internal.WebApi/Commands/ActivityContextCommand.cs:L1-10
<img border=0 src='https://avatars.githubusercontent.com/u/6068068?v=3' height=16 width=16'> Command Context is IDisposable? Indeed? Please describe case.
<img border=0 src='https://avatars.githubusercontent.com/u/10926945?v=3' height=16 width=16'> If I want to register type in OwinContext it has to implement IDisposable
<img border=0 src='https://avatars.githubusercontent.com/u/6068068?v=3' height=16 width=16'> You really needed OWIN context? OWIN in our case it's like DI as SimpleInjector. I suggest that you can you use SimpleInjector instead.
<img border=0 src='https://avatars.githubusercontent.com/u/10926945?v=3' height=16 width=16'> I'll fix it
[ ] File: DevelopGitHub/GitHubExtension/GitHubExtension.Infrastructure.ActionFilters/InternalActivitiesFilters/InternalActivityFilter.cs:L1-7
<img border=0 src='https://avatars.githubusercontent.com/u/6068068?v=3' height=16 width=16'> Abstract class without any line of code?
<img border=0 src='https://avatars.githubusercontent.com/u/10926945?v=3' height=16 width=16'> will change to interface wich marked my attributes public interface IInternalActivityFilter { }
[ ] File: DevelopGitHub/GitHubExtension/GitHubExtension.Infrastructure.ActionFilters/InternalActivitiesFilters/AssignRoleToUserActivityAttribute.cs:L1-59
<img border=0 src='https://avatars.githubusercontent.com/u/6068068?v=3' height=16 width=16'> Where interfaces? Why you are using concrete types ActivityContextCommand, ActivityContextQuery ?
<img border=0 src='https://avatars.githubusercontent.com/u/10926945?v=3' height=16 width=16'> I'll fix it
[ ] File: DevelopGitHub/GitHubExtension/GitHubExtension.Infrastructure.ActionFilters/Extensions/ActivityFiltersExtensions.cs:L1-55
<img border=0 src='https://avatars.githubusercontent.com/u/6068068?v=3' height=16 width=16'> Can you reso;ve all dependencies through the SimpleInjector intead of spreading resolving instances in different places? You you should have only one point of injection and resolving.
<img border=0 src='https://avatars.githubusercontent.com/u/10926945?v=3' height=16 width=16'> when injecting dependencies into Web API filter attributes, get my instances from dependency resolver http://simpleinjector.readthedocs.io/en/latest/webapiintegration.html , according to this documentation. For example: var checker = GlobalConfiguration.Configuration.DependencyResolver .GetService(typeof(IMinimumAgeChecker)) as IMinimumAgeChecker;
[ ] File: DevelopGitHub/GitHubExtension/GitHubExtension.Infrastructure.ActionFilters/Extensions/ActivityFiltersExtensions.cs:L1-55
<img border=0 src='https://avatars.githubusercontent.com/u/6068068?v=3' height=16 width=16'> You are using this method only in one place. Why it should be public? Incapsulate all unnessesary information that is not available for client.
<img border=0 src='https://avatars.githubusercontent.com/u/10926945?v=3' height=16 width=16'> Already done
[ ] File: DevelopGitHub/GitHubExtension/GitHubExtension.Infrastructure.ActionFilters/Extensions/ActivityFiltersExtensions.cs:L1-55
<img border=0 src='https://avatars.githubusercontent.com/u/6068068?v=3' height=16 width=16'> Same.
<img border=0 src='https://avatars.githubusercontent.com/u/10926945?v=3' height=16 width=16'> already done
[ ] File: DevelopGitHub/GitHubExtension/GitHubExtension.Infrastructure.Extensions/Identity/CurrentProjectExtensions.cs:L1-25
<img border=0 src='https://avatars.githubusercontent.com/u/6068068?v=3' height=16 width=16'> Is it correct name for class holding extension methods for IPrincipal interface?
<img border=0 src='https://avatars.githubusercontent.com/u/10926945?v=3' height=16 width=16'> No. Correct name is IdentityExtensions
<img border=0 src='https://avatars.githubusercontent.com/u/10926945?v=3' height=16 width=16'> I'll fix it