PartTemplate work #293
PartTemplate work #293
Conversation
RossStewart
requested review from
lewisrenfrew,
richardiphillips,
andrewfraser73,
iaincrawf and
a team
and removed request for
lewisrenfrew
| public class PartTemplateResponseProcessor : JsonResponseProcessor<PartTemplate> | ||
| { | ||
| public PartTemplateResponseProcessor(IResourceBuilder<PartTemplate> resourceBuilder) | ||
| : base(resourceBuilder, "linnapps-part-templates", 1) |
There was a problem hiding this comment.
this should be the singular. "linnapps-part-template"
| [SetUp] | ||
| public void SetUp() | ||
| { | ||
| var a = new PartTemplate |
There was a problem hiding this comment.
i'd use a more descriptive variable name than a
| resource.PartRoot.Should().Be("PARTTEMP"); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
we tend to add a newline at the end of files to stop github complaining at us
src/Service/Modules/PartsModule.cs
Outdated
| var result = this.partTemplateService.Add(resource); | ||
| return this.Negotiate.WithStatusCode(HttpStatusCode.Created) | ||
| .WithModel(result) | ||
| .WithMediaRangeModel("text/html", ApplicationSettings.Get) |
There was a problem hiding this comment.
you don't need to handle html here because it will never be requested in a PUT or a POST
| @@ -0,0 +1,59 @@ | |||
| namespace Linn.Stores.Service.Tests.PartsModuleSpecs | |||
| { | |||
| using System.Collections.Generic; | |||
There was a problem hiding this comment.
are these usings needed for the get by id?
| { | ||
| var a = new PartTemplate | ||
| { | ||
| PartRoot = "PARTTEMP" |
There was a problem hiding this comment.
it might be nice to declare "PARTTEMP" into a string field and then you can reference it against all the tests which avoids typos and also makes it clear what's being tested
| AccountingCompany = model.AccountingCompany, | ||
| ProductCode = model.ProductCode, | ||
| StockControlled = model.StockControlled, | ||
| LinnProduced = model.LinnProduced, | ||
| RmFg = model.RmFg, | ||
| BomType = model.BomType, | ||
| AssemblyTechnology = model.AssemblyTechnology, | ||
| AllowPartCreation = model.AllowPartCreation, |
There was a problem hiding this comment.
remember you can set the "self" linkrel in GetLocation down below
| @@ -35,7 +35,7 @@ public IQueryable<PartTemplate> FindAll() | |||
|
|
|||
There was a problem hiding this comment.
remember to take off AsNoTracking up above if we're amending these things now
|
No worries, was going to add a few more tests on Monday morning so I’ll make these cha ges then too! |
| @@ -6,13 +6,16 @@ import partsSelectors from '../../selectors/partsSelectors'; | |||
| import { getPrivileges } from '../../selectors/userSelectors'; | |||
| import partTemplatesActions from '../../actions/partTemplatesActions'; | |||
| import partTemplatesSelectors from '../../selectors/partTemplatesSelectors'; | |||
| import partTemplateActions from '../../actions/partTemplatesActions'; | |||
| import partTemplateSelectors from '../../selectors/partTemplatesSelectors'; | |||
|
|
|||
There was a problem hiding this comment.
do we need these extra action creators and selectors in the parts container?
There was a problem hiding this comment.
I also just realised I never got you set up witht the linter which would have complained about this most likely. Give me a shout today and I can try and remember what we need to do to get it working
There was a problem hiding this comment.
Can talk about it after stand up if you're about
PR for PartTemplate work. Initial backend submitted more submissions and tests to follow.