OBPIH-6166 Add endpoint for create/edit product package#4522
OBPIH-6166 Add endpoint for create/edit product package#4522awalkowiak merged 10 commits intofeature/product-supplier-list-redesignfrom
Conversation
…ion, adjust toJson() with new needed properties, adjust defaultProductPackageDerived to current behavior of toJson()
kchelstowski
requested review from
alannadolny,
awalkowiak,
drodzewicz and
jmiranda
| import org.pih.warehouse.core.ProductPrice | ||
|
|
||
| @Transactional | ||
| class ProductPackageService { |
There was a problem hiding this comment.
I feel somewhat weird about creating product package controller and service, because anyway I have to return ProductSupplier from the save method (because I mutate the product supplier also, e.g. minOrderQuantity), so I don't know if it's a "REST-y" solution to create a separate controller/service for this case if I do not return the ProductPackage, or if it should be handled in the product supplier service.
But since we've discussed to create its own API for that, I decided to go with that for now, but I can be easily convinced to move it to the ProductSupplierService and rename the methods.
Let me know what you think.
| // If product package price is not provided, skip assigning pricing data | ||
| if (!command.productPackagePrice) { | ||
| return | ||
| } |
There was a problem hiding this comment.
this would mean that if we set the price once, we can't make it null anymore. But this is how it is done currently in the import, hence I kept the current behavior.
There was a problem hiding this comment.
Actually, it can't become null, but it can still be 0 which it can't with my solution, because I check the truthiness. I'm gonna change it to == null
…truthiness to check if it's null.
kchelstowski
force-pushed
the
OBPIH-6166
branch
from
8068c83 to
3faa039
Compare
|
|
||
| ProductPackage getDefaultProductPackage() { | ||
| ProductPackage getDefaultProductPackageDerived() { | ||
| return productPackages ? productPackages.sort { it.lastUpdated }.last() : null |
There was a problem hiding this comment.
maybe it is worth to create compareTo in the ProductPackage domain
There was a problem hiding this comment.
I just renamed the method, didn’t care about the content, because it was not in the scope of my work
There was a problem hiding this comment.
Oh, that's true, didn't notice that, sry
| return command.productSupplier | ||
| } | ||
|
|
||
| private static void setPackageData(ProductPackageCommand command) { |
There was a problem hiding this comment.
why is this method static (and the one below also)?
There was a problem hiding this comment.
Because the implementation doesn’t depend on actual instance of the service
There was a problem hiding this comment.
setPackageData sounds like a method that should set something on a currently existing instance, something feels wrong to me
There was a problem hiding this comment.
it does on the product supplier instance that is bound to the command.
| if (date < minDate) { | ||
| return false | ||
| } | ||
| return true |
There was a problem hiding this comment.
can't you just return date < minDate?
There was a problem hiding this comment.
I probably could but would have to negate it, because otherwise I’d return true if date < minDate
| } | ||
| return 0.0 | ||
| } | ||
|
|
There was a problem hiding this comment.
lines 133-172 (and this commit in particular fd98c02) are applying the new behavior of the default product package to the product sources list.
At first we wanna search for a default product package that is set in the new association ProductPackage defaultProductPackage and if we do not find one, then we wanna search for the default product package "in an old way" to support backwards compatibility.
It has been specified and requested in the comments of OBPIH-6116.
* OBPIH-6166 Change current defaultProductPackge to defaultProductPackageDerived * OBPIH-6166 Add defaultProductPackage column to product supplier table * OBPIH-6166 Adjust domain changes - add defaultProductPackage association, adjust toJson() with new needed properties, adjust defaultProductPackageDerived to current behavior of toJson() * OBPIH-6166 Adjust frontend pricing section to pricing endpoint * OBPIH-6166 Add product package toJson() * OBPIH-6166 Add endpoint for create/edit product package with pricing * OBPIH-6166 Change checking the productPackagePrice from checking the truthiness to check if it's null. * OBPIH-6166 Shorten the validator for contractPriceValidUntil date * OBPIH-6166 Apply the new behavior of default product package to the product supplier list * OBPIH-6166 Fix a bug related to passing an empty contractPriceValidUntil
3 participants
No description provided.