Subtracted value on interim contract #17931
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ModCntr_Settings: interestrate & addinterestdays ModCntr_InvoicingGroup: totalinterest Moved ModCntr_InvoicingGroup & ModCntr_InvoicingGroup_Product to contracts
mrie04
reviewed
Apr 24, 2024
| .uomId(uomId) | ||
| .build(); | ||
|
|
||
| final I_C_Flatrate_Term interimContractRecord = flatrateBL.getById(createLogRequest.getContractId()); |
There was a problem hiding this comment.
pls check changes from https://github.com/metasfresh/metasfresh/pull/17945/files
…lence_uat_SVInterimContract # Conflicts: # backend/de.metas.contracts/src/main/java/de/metas/contracts/modular/computing/purchasecontract/interim/InterimInvoiceLineLog.java # backend/de.metas.contracts/src/main/java/de/metas/contracts/modular/computing/purchasecontract/subtractedvalue/interim/SVInterimComputingMethod.java
…lence_uat_SVInterimContract # Conflicts: # backend/de.metas.contracts/src/main/java-gen/de/metas/contracts/model/X_ModCntr_Log.java # backend/de.metas.contracts/src/main/java-gen/de/metas/contracts/model/X_ModCntr_Settings.java # backend/de.metas.contracts/src/main/java/de/metas/contracts/modular/ModularContractPriceService.java # backend/de.metas.contracts/src/main/java/de/metas/contracts/modular/computing/purchasecontract/interim/InterimInvoiceLineLog.java # backend/de.metas.contracts/src/main/java/de/metas/contracts/modular/computing/purchasecontract/subtractedvalue/interim/SVInterimComputingMethod.java # backend/de.metas.contracts/src/main/java/de/metas/contracts/modular/log/ModularContractLogDAO.java
…mns to windows in Invoicing Group and Modular contract settings
…lence_uat_SVInterimContract
- Removed `Balance` - Added `C_Currency_ID` - Renamed `Interest` to `InterestScore`
Changed ModCntr_Compute_Interest process to: - default p_InterestToDistribute & p_InterestToDistributeCurrencyId based on currently selected p_InvoicingGroupId - support no selection on ModCntr_Compute_Interest - Added ModCntr_Compute_Interest to menu and added missing params
teosarca
reviewed
May 16, 2024
| @Override | ||
| public boolean applies(final @NonNull TableRecordReference recordRef, @NonNull final LogEntryContractType logEntryContractType) | ||
| { | ||
| if (logEntryContractType.isModularContractType() && recordRef.getTableName().equals(I_M_Shipping_NotificationLine.Table_Name)) |
There was a problem hiding this comment.
minor: we have .equalsByTableName
teosarca
reviewed
May 16, 2024
| Check.assumeNotNull(invoice.getAmount(), "Invoices with no amount should've been skipped already! LogId=" | ||
| + invoice.getId()); | ||
|
|
||
| final Instant conversionDate = Instant.now(); |
There was a problem hiding this comment.
pls avoid using Instant.now()!
pls introduce the run date as a field in InterestBonusComputationRequest.
More, you might want to refactor all code from this Service into some InterestBonusComputationCommand.
teosarca
reviewed
May 16, 2024
| } | ||
|
|
||
| @NonNull | ||
| public synchronized AllocationItem allocate(@NonNull final AllocationItem shippingNotification) |
There was a problem hiding this comment.
why these methods are synchronized?
There was a problem hiding this comment.
bc I'm chaning its state, I want to make sure if at any given point this class ends up being used in a concurrent manner, it won't get messy
teosarca
reviewed
May 16, 2024
| .toBigDecimal() | ||
| .multiply(bonusInterestRate) | ||
| .multiply(NumberUtils.asBigDecimal(interestDays)) | ||
| .divide(NumberUtils.asBigDecimal(360), RoundingMode.HALF_UP); |
There was a problem hiding this comment.
pls use BigDecimal.valueOf; extract it as constant and document why is 360
teosarca
reviewed
May 16, 2024
|
|
||
| if (currentInvoiceAllocations.canAllocate(currentShippingNotification)) | ||
| { | ||
| currentShippingNotification = currentInvoiceAllocations.allocate(currentShippingNotification); |
There was a problem hiding this comment.
pls check, currentShippingNotification might be null
There was a problem hiding this comment.
it can't get pass canAllocate if it's null
teosarca
reviewed
May 16, 2024
| @@ -145,24 +141,16 @@ public ComputingResponse compute(final @NonNull ComputingRequest request) | |||
| .contractModuleId(request.getModularContractModuleId()) | |||
| .lockOwner(request.getLockOwner()) | |||
| .build(); | |||
| final ModularContractLogEntriesList modularContractLogEntries = modularContractLogService.getModularContractLogEntries(query); | |||
| if (modularContractLogEntries.isEmpty()) | |||
| final PInstanceId pInstanceId = modularContractLogService.getModularContractLogEntrySelection(query); | |||
There was a problem hiding this comment.
super minor: IMHO u can call it selectionId instead pInstanceId (which sounds weird) :)
teosarca
reviewed
May 16, 2024
| @@ -321,7 +322,7 @@ private InvoiceAllocations initInvoiceAllocations( | |||
| Check.assumeNotNull(invoice.getAmount(), "Invoices with no amount should've been skipped already! LogId=" | |||
| + invoice.getId()); | |||
|
|
|||
| final Instant conversionDate = Instant.now(); | |||
| final Instant conversionDate = SystemTime.asInstant(); | |||
There was a problem hiding this comment.
SystemTime.asInstant() is way better than Instant.now().... but pls try avoiding it as much as possible.
What stops you from getting the "computingDate" from request/param?
…act' into inner_silence_uat_SVInterimContract # Conflicts: # backend/de.metas.contracts/src/main/java/de/metas/contracts/modular/interest/InterestComputationWorkPackageProcessor.java
teosarca
reviewed
May 16, 2024
| public static final int TOTAL_DAYS_OF_FISCAL_YEAR = 360; | ||
| private final ModularLogInterestRepository interestRepository = SpringContextHolder.instance.getBean(ModularLogInterestRepository.class); | ||
| private final ModularContractLogService modularContractLogService = SpringContextHolder.instance.getBean(ModularContractLogService.class); | ||
| private final MoneyService moneyService = SpringContextHolder.instance.getBean(MoneyService.class); |
There was a problem hiding this comment.
pls avoid getBean. U can have those as ctor params
teosarca
reviewed
May 16, 2024
| @@ -53,7 +56,7 @@ public static BigDecimal computeScore(@NonNull final Money amount, final int int | |||
| { | |||
| final BigDecimal amountAsBD = amount.toBigDecimal(); | |||
|
|
|||
| return amountAsBD.multiply(NumberUtils.asBigDecimal(interestDays)) | |||
| .divide(NumberUtils.asBigDecimal(100), amountAsBD.scale(), RoundingMode.HALF_UP); | |||
| return amountAsBD.multiply(BigDecimal.valueOf(interestDays)) | |||
There was a problem hiding this comment.
this looks like a percentage :)
There was a problem hiding this comment.
Looks, I assure you it is not :)
teosarca
reviewed
May 16, 2024
| @@ -106,7 +106,7 @@ private void computeAndDistributeInterest(@NonNull final InterestComputationRequ | |||
|
|
|||
| final InitialInterestAllocationResult result = createInterestRecords(request); | |||
| final BigDecimal interestDistributionPercent = request.getInterestToDistribute().toBigDecimal() | |||
| .divide(result.getTotalInterestScore(), result.getTotalInterestScore().scale(), RoundingMode.HALF_UP); | |||
| .divide(result.getTotalInterestScore(), request.getInterestScale(), RoundingMode.HALF_UP); | |||
teosarca
reviewed
May 16, 2024
| @@ -41,6 +41,7 @@ public class InterestComputationRequest | |||
| @NonNull Money interestToDistribute; | |||
| @NonNull LockOwner lockOwner; | |||
| @NonNull ComputingMethodType computingMethodType; | |||
| @NonNull Integer interestScale; | |||
There was a problem hiding this comment.
shall we use CurrencyPrecision instead of Integer?
(TBH not sure what interestScale means)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#18016