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 #90

Closed
Closed

Conversation

@mfirmin1
Copy link

mfirmin1 commented Feb 4, 2020

No description provided.

@mfirmin1

This comment has been minimized.

Copy link
Author

mfirmin1 commented Feb 4, 2020

Also low commits because I am terrible at remembering to commit while working on an assignment

@mfirmin1 mfirmin1 changed the title Not done with test Assignment4 Feb 4, 2020
@JeffreyNix

This comment has been minimized.

Copy link

JeffreyNix commented Feb 6, 2020

Unofficial review:
SecretSanta.Api project file needs:

  <PropertyGroup>
    <IncludeOpenAPIAnalyzers>true</IncludeOpenAPIAnalyzers>
  </PropertyGroup>

to enable the webapi handlers

EntityController can be abstract and probably should be. also there's no http verb on post and it doesn't appear to be returning one (like ok or not found)

tests need to test controller.get(), and controller.post() tests.

Copy link
Collaborator

Keboo left a comment

(mfirmin1)

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.Analyzers, Microsoft.CodeAnalysis.CSharp.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.Analyzers, Microsoft.CodeAnalysis.CSharp.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.Analyzers, Microsoft.CodeAnalysis.CSharp.Analyzers, Microsoft.CodeAnalysis.VersionCheckAnalyzer, Microsoft.EntityFrameworkCore.Analyzers 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.Analyzers, Microsoft.CodeAnalysis.CSharp.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.Analyzers, Microsoft.CodeAnalysis.CSharp.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.Analyzers, Microsoft.CodeAnalysis.CSharp.Analyzers, Microsoft.CodeAnalysis.VersionCheckAnalyzer, Microsoft.EntityFrameworkCore.Analyzers 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 ✔/ - Mostly done only a couple tests that were not done/working

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<TEntity>> Put(int id, TEntity entity)
{
if (await EntityService.UpdateAsync(id, entity) is TEntity value)

This comment has been minimized.

Copy link
@Keboo

Keboo Feb 11, 2020

Collaborator

Local variable 'value' should be returned rather than entity.

using SecretSanta.Business;
using SecretSanta.Business.Services;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.EntityFrameworkCore;*/

This comment has been minimized.

Copy link
@Keboo

Keboo Feb 11, 2020

Collaborator

Why comment all of this out? In general you should favor removing code rather than commenting it out.

<PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="3.1.1" />
<PackageReference Include="Microsoft.Extensions.Logging.Debug" Version="3.1.1" />
<PackageReference Include="Microsoft.VisualStudio.Web.CodeGeneration.Design" Version="3.1.1" />
<PackageReference Include="NSwag.AspNetCore" Version="13.2.2" />

This comment has been minimized.

Copy link
@Keboo

Keboo Feb 11, 2020

Collaborator

This NuGet should only be added to web projects.

public void Create_GiftController_Success()
{
//Arrange
//Act

This comment has been minimized.

Copy link
@Keboo

Keboo Feb 11, 2020

Collaborator

Duplicate comments?

//Arrange
//Act
//Assert
var service = new GroupTestService();

This comment has been minimized.

Copy link
@Keboo

Keboo Feb 11, 2020

Collaborator

Technically this is the "Act" portion.

/*
* Couldn't quite get the Post.
[TestMethod]
public async Task Post_User_Sucess()

This comment has been minimized.

Copy link
@Keboo

Keboo Feb 11, 2020

Collaborator

Take a look at my Assignment4Complete solution branch for an example.

@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.