Skip to content
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

Assignment4 #91

Closed
Closed

Conversation

@JeffreyNix
Copy link

JeffreyNix commented Feb 4, 2020

Finishing up Put test but everything else should be there

@CDobbins1

This comment has been minimized.

Copy link

CDobbins1 commented Feb 6, 2020

Reviewing this

Copy link

CDobbins1 left a comment

Assignment

  • WebAPI Analyzers enabled ✔
  • DI properly configured
    • Services properly registered with correct scopes ✔
    • DB context properly registered ✔
    • AutoMapper properly registered ✔
  • NSwag properly setup ✔
  • API controllers properly expose CRUD operations ✔
  • Controllers are all unit tested ✔

Extra Credit

  • Unit tests in EntityServiceTests fixed approprietly
  • Mocking framework used to create the test doubles

namespace SecretSanta.Api.Controllers
{
public interface IController<TEntity>

This comment has been minimized.

Copy link
@CDobbins1

CDobbins1 Feb 6, 2020

Since the classes implementing this interface all have common code, consider making this an abstract class so the individual controllers can inherit functionality.

This comment has been minimized.

Copy link
@JeffreyNix

JeffreyNix Feb 6, 2020

Author

yeah, I really should migrate things over to that. it was a quick hack to try and make things work generically for unit tests and I never got back to it.

Copy link
Collaborator

Keboo left a comment

(JeffreyNix)

Assignment4

Project Analysis Summary
Nullable Enabled:
Unexpected Disabled Warnings:
Appropriately handle all warnings:
ProjectName Nullable Solutions Analyzers Unexpected Disabled Warnings Waning Level
SecretSanta.Api Enable SecretSanta.sln IntelliTect Analyzers, Microsoft CodeQuality Analyzers, Microsoft .NET Core Analyzers, Microsoft .NET Framework Analyzers, Microsoft.CodeAnalysis.Analyzers, Microsoft.CodeAnalysis.CSharp.Analyzers, Microsoft.CodeAnalysis.VersionCheckAnalyzer, Microsoft.EntityFrameworkCore.Analyzers 4
SecretSanta.Business Enable SecretSanta.sln IntelliTect Analyzers, Microsoft CodeQuality Analyzers, Microsoft .NET Core Analyzers, Microsoft .NET Framework Analyzers, Microsoft.CodeAnalysis.VersionCheckAnalyzer, Microsoft.EntityFrameworkCore.Analyzers 4
SecretSanta.Data Enable SecretSanta.sln IntelliTect Analyzers, Microsoft CodeQuality Analyzers, Microsoft .NET Core Analyzers, Microsoft .NET Framework Analyzers, Microsoft.CodeAnalysis.VersionCheckAnalyzer, Microsoft.EntityFrameworkCore.Analyzers 4
SecretSanta.Web Enable SecretSanta.sln IntelliTect Analyzers, Microsoft CodeQuality Analyzers, Microsoft .NET Core Analyzers, Microsoft .NET Framework Analyzers, Microsoft.CodeAnalysis.VersionCheckAnalyzer 4
SecretSanta.Api.Tests Enable SecretSanta.sln IntelliTect Analyzers, Microsoft CodeQuality Analyzers, Microsoft .NET Core Analyzers, Microsoft .NET Framework Analyzers, Microsoft.CodeAnalysis.Analyzers, Microsoft.CodeAnalysis.CSharp.Analyzers, Microsoft.CodeAnalysis.VersionCheckAnalyzer, Microsoft.EntityFrameworkCore.Analyzers 4
SecretSanta.Business.Tests Enable SecretSanta.sln IntelliTect Analyzers, Microsoft CodeQuality Analyzers, Microsoft .NET Core Analyzers, Microsoft .NET Framework Analyzers, Microsoft.CodeAnalysis.VersionCheckAnalyzer, Microsoft.EntityFrameworkCore.Analyzers 4
SecretSanta.Data.Tests Enable SecretSanta.sln IntelliTect Analyzers, Microsoft CodeQuality Analyzers, Microsoft .NET Core Analyzers, Microsoft .NET Framework Analyzers, Microsoft.CodeAnalysis.VersionCheckAnalyzer, Microsoft.EntityFrameworkCore.Analyzers 4
SecretSanta.Web.Tests Enable SecretSanta.sln IntelliTect Analyzers, Microsoft CodeQuality Analyzers, Microsoft .NET Core Analyzers, Microsoft .NET Framework Analyzers, Microsoft.CodeAnalysis.VersionCheckAnalyzer 4

Main Assignment

  • WebAPI Analyzers enabled ✔
  • DI properly configured ✔
  • Services properly registered with correct scopes ✔
  • DB context properly registered
  • AutoMapper properly registered ✔
  • NSwag properly setup ✔
  • API controllers properly expose CRUD operations ✔
  • Controllers are all unit tested ✔

Extra Credit

  • Unit tests in EntityServiceTests fixed appropriately
  • Mocking framework used to create the test doubles

Peer Review

  • On time ✔
  • Accurate ✔
  • Appropriate comments given ✔
[ProducesResponseType(StatusCodes.Status404NotFound)]
public async Task<ActionResult<bool>> Delete(int id)
{
if (await GiftService.FetchByIdAsync(id) is { } gift)

This comment has been minimized.

Copy link
@Keboo

Keboo Feb 11, 2020

Collaborator

Local variable 'gift' is never used.

//}
public static void ConfigureServices(IServiceCollection services)
{
services.AddDbContext<ApplicationDbContext>();

This comment has been minimized.

Copy link
@Keboo

Keboo Feb 11, 2020

Collaborator

Though this technically registers the ApplicationDbContext type, it does not actually configure the db context and will fail on all DB interaction.

services.AddScoped<IUserService, UserService>();
services.AddScoped<IGiftService, GiftService>();

services.AddScoped<UserController, UserController>();

This comment has been minimized.

Copy link
@Keboo

Keboo Feb 11, 2020

Collaborator

You should not need to register the classes directly. All usages should depend on the interface abstraction and not the concrete type.

@Keboo Keboo closed this Feb 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.