-
Notifications
You must be signed in to change notification settings - Fork 24
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
Allow Products with multiple projects #3578
Conversation
✅ No secrets were detected in the code. |
Codecov Report
@@ Coverage Diff @@
## dev #3578 +/- ##
==========================================
- Coverage 69.59% 69.57% -0.03%
==========================================
Files 1370 1371 +1
Lines 33751 33859 +108
Branches 6283 6290 +7
==========================================
+ Hits 23489 23557 +68
- Misses 10011 10049 +38
- Partials 251 253 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
✅ No secrets were detected in the code. |
✅ No secrets were detected in the code. |
tested h179, h120, and h0774 and h0443 form templates to ensure project printed correctly, appears to be working. |
Also tested project export, looks good. |
also tested alternate project and acquisition list view project search, that is working. |
project advanced search also working. |
.Inherits<Entity.IBaseEntity, BaseModel>(); | ||
|
||
config.NewConfig<ProjectProductModel, Entity.PimsProjectProduct>() | ||
.Map(dest => dest.ProjectProductId, src => src.Id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems like an unusual mixture of fields here (one id and then one entity), is there a reason why that is necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assumed the id would be coming on the object. I've added the id from the field as well
@@ -113,11 +115,11 @@ public IActionResult GetAll() | |||
[ProducesResponseType(typeof(ProjectModel), 200)] | |||
[ProducesResponseType(typeof(Api.Models.ErrorResponseModel), 400)] | |||
[SwaggerOperation(Tags = new[] { "project" })] | |||
public IActionResult AddProject(ProjectModel projectModel) | |||
public IActionResult AddProject(ProjectModel projectModel, [FromQuery] string[] userOverrideCodes) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what happens if the frontend doesn't provide this query param? does the controller interpret this as an empty array?
Updates the passed project with the matched products in place. | ||
Note: The return list contains the external products matched | ||
*/ | ||
private List<PimsProduct> MatchProducts(PimsProject project) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function does seem like one we should have good test coverage on (via the public method).
@FuriousLlama the estimate dates on products to not appear to work. |
✅ No secrets were detected in the code. |
✅ No secrets were detected in the code. |
✅ No secrets were detected in the code. |
✅ No secrets were detected in the code. |
No description provided.