You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2020/05/02 18:53:19 UTC
[GitHub] [fineract] vorburger commented on a change in pull request #796: FINERACT-38
vorburger commented on a change in pull request #796:
URL: https://github.com/apache/fineract/pull/796#discussion_r418993637
##########
File path: fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/JournalEntryWritePlatformServiceJpaRepositoryImpl.java
##########
@@ -625,6 +622,63 @@ private void saveAllDebitOrCreditEntries(final JournalEntryCommand command, fina
}
}
+ @Transactional
+ @Override
+ public String createJournalEntryForIncomeAndExpenseBookOff(final JournalEntryCommand journalEntryCommand) {
+ try{
+ journalEntryCommand.validateForCreate();
+
+ // check office is valid
+ final Long officeId = journalEntryCommand.getOfficeId();
+ final Office office = this.officeRepositoryWrapper.findOneWithNotFoundDetection(officeId);
+
+ final String currencyCode = journalEntryCommand.getCurrencyCode();
+
+ validateBusinessRulesForJournalEntries(journalEntryCommand);
+ /** Capture payment details **/
+ final PaymentDetail paymentDetail =null;
+
+ /** Set a transaction Id and save these Journal entries **/
+ final Date transactionDate = journalEntryCommand.getTransactionDate().toDate();
+ final String transactionId = generateTransactionId(officeId);
+ final String referenceNumber = journalEntryCommand.getReferenceNumber();
+
+ debitOrCreditEntriesForIncomeAndExpenseBooking(journalEntryCommand, office, paymentDetail, currencyCode, transactionDate,
+ journalEntryCommand.getDebits(), transactionId, JournalEntryType.DEBIT, referenceNumber);
+
+ debitOrCreditEntriesForIncomeAndExpenseBooking(journalEntryCommand, office, paymentDetail, currencyCode, transactionDate,
+ journalEntryCommand.getCredits(), transactionId, JournalEntryType.CREDIT, referenceNumber);
+
+ return transactionId;
+ }catch (final DataIntegrityViolationException dve) {
+ handleJournalEntryDataIntegrityIssues(dve);
+ return null;
Review comment:
This confused me while I was doing a technical code review of this PR, so I had a closer look at the existing code when I realized that this was just copy/pasted... it's already wrong in that existing code, I'm proposing to improve it in https://github.com/apache/fineract/pull/799.. would you be willing to rebase and change this accordingly, once that is merged?
##########
File path: fineract-provider/src/main/java/org/apache/fineract/organisation/office/service/OfficeReadPlatformServiceImpl.java
##########
@@ -266,4 +266,20 @@ public OfficeTransactionData retrieveNewOfficeTransactionDetails() {
public PlatformSecurityContext getContext() {
return this.context;
}
+
+ @Override
+ public Collection<Long> officeByHierarchy(Long officeId) {
+ StringBuilder sqlBuilder = new StringBuilder();
+ sqlBuilder.append("SELECT ounder.id FROM m_office mo ");
+ sqlBuilder.append(" JOIN m_office ounder ON ounder.hierarchy LIKE CONCAT(mo.hierarchy, '%') ");
+ sqlBuilder.append(" AND ounder.hierarchy like CONCAT('.', '%') where mo.id = ? ");
+
+
+ try {
+ return this.jdbcTemplate.queryForList(sqlBuilder.toString(), Long.class,
+ new Object[] { officeId});
+ } catch (final EmptyResultDataAccessException e) {
+ return null;
Review comment:
safer to return a `Collections.emptyList()` instead of `null` here
##########
File path: fineract-provider/src/main/java/org/apache/fineract/accounting/closure/api/GLClosuresApiResource.java
##########
@@ -143,6 +159,21 @@ public String createGLClosure(@ApiParam(hidden = true) final String jsonRequestB
return this.apiJsonSerializerService.serialize(result);
}
+ @POST
+ @Path("previewIncomeAndExpense")
+ @Consumes({ MediaType.APPLICATION_JSON })
+ @Produces({ MediaType.APPLICATION_JSON })
+ @ApiOperation(value = "Create an End of the Year Accounting Closure")
+ public String previewIncomeAndExpenseBooking(@Context final UriInfo uriInfo, final String apiRequestBodyAsJson) {
+ final JsonElement parsedQuery = this.fromJsonHelper.parse(apiRequestBodyAsJson);
+ final JsonQuery query = JsonQuery.from(apiRequestBodyAsJson, parsedQuery, this.fromJsonHelper);
+ final Collection<IncomeAndExpenseBookingData> incomeAndExpenseBookingCollection = this.calculateIncomeAndExpenseBooking.CalculateIncomeAndExpenseBookings(query);
+ final ApiRequestJsonSerializationSettings settings = this.apiRequestParameterHelper.process(uriInfo.getQueryParameters());
+ return this.incomeAndExpenseBookingDataDefaultToApiJsonSerializer.serialize(settings, incomeAndExpenseBookingCollection, new HashSet<String>());
Review comment:
safer to use a `Collections.emptySet()` instead of `new HashSet<String>()` here
##########
File path: fineract-provider/src/main/java/org/apache/fineract/accounting/closure/service/GLClosureWritePlatformServiceJpaRepositoryImpl.java
##########
@@ -148,4 +261,99 @@ private void handleGLClosureIntegrityIssues(final JsonCommand command, final Dat
throw new PlatformDataIntegrityException("error.msg.glClosure.unknown.data.integrity.issue",
"Unknown data integrity issue with resource GL Closure: " + realCause.getMessage());
}
+ private void handleRunningBalanceNotCalculated(final HashMap<Long,List<IncomeAndExpenseJournalEntryData>> allIncomeAndExpenseJLWithSubBranches){
+ final Iterator<Map.Entry<Long,List<IncomeAndExpenseJournalEntryData>>> iterator = allIncomeAndExpenseJLWithSubBranches.entrySet().iterator();
+ while(iterator.hasNext()){
+ final List<IncomeAndExpenseJournalEntryData> incomeAndExpenseJL = iterator.next().getValue();
+ for(final IncomeAndExpenseJournalEntryData incomeAndExpenseData :incomeAndExpenseJL ){
+ if(!incomeAndExpenseData.isRunningBalanceCalculated()){ throw new RunningBalanceNotCalculatedException(incomeAndExpenseData.getOfficeId());}
+ }
+ }
+ }
+ private String bookOffIncomeAndExpense(final List<IncomeAndExpenseJournalEntryData> incomeAndExpenseJournalEntryDataList,
+ final GLClosureCommand closureData,final GLAccount glAccount, final Office office){
+ /* All running balances has to be calculated before booking off income and expense account */
+ for(final IncomeAndExpenseJournalEntryData incomeAndExpenseData :incomeAndExpenseJournalEntryDataList ){
+ if(!incomeAndExpenseData.isRunningBalanceCalculated()){ throw new RunningBalanceNotCalculatedException(incomeAndExpenseData.getOfficeId());}
+ }
+ BigDecimal debits = BigDecimal.ZERO;
+ BigDecimal credits = BigDecimal.ZERO;
+
+ int i = 0;
+ int j = 0;
+ for(final IncomeAndExpenseJournalEntryData incomeAndExpense : incomeAndExpenseJournalEntryDataList){
+ if(incomeAndExpense.isIncomeAccountType()){
+ if(incomeAndExpense.getOfficeRunningBalance().signum() == 1){debits = debits.add(incomeAndExpense.getOfficeRunningBalance());
+ i++;
+ }
+ else{ credits = credits.add(incomeAndExpense.getOfficeRunningBalance().abs());
+ j++;
+ }
+ }
+ if(incomeAndExpense.isExpenseAccountType()){
+ if(incomeAndExpense.getOfficeRunningBalance().signum() == 1){credits = credits.add(incomeAndExpense.getOfficeRunningBalance());
+ j++;
+ }
+ else{debits= debits.add(incomeAndExpense.getOfficeRunningBalance().abs());
+ i++;
+ }
+ }
+ }
+ final int compare = debits.compareTo(credits);
+ BigDecimal difference = BigDecimal.ZERO;
+ JournalEntryCommand journalEntryCommand = null;
+ if(compare ==1){ j++;}else{
+ i++;
+ }
+
+ SingleDebitOrCreditEntryCommand[] debitsJournalEntry = new SingleDebitOrCreditEntryCommand[i];
+ SingleDebitOrCreditEntryCommand[] creditsJournalEntry = new SingleDebitOrCreditEntryCommand[j];
+ int m=0;
+ int n=0;
+ for(final IncomeAndExpenseJournalEntryData incomeAndExpense : incomeAndExpenseJournalEntryDataList){
+ if(incomeAndExpense.isIncomeAccountType()){
+ if(incomeAndExpense.getOfficeRunningBalance().signum() == 1){
+ debitsJournalEntry[m] = new SingleDebitOrCreditEntryCommand(null,incomeAndExpense.getAccountId(),incomeAndExpense.getOfficeRunningBalance().abs(),null);
+ m++;
+ }else{
+ creditsJournalEntry[n]= new SingleDebitOrCreditEntryCommand(null,incomeAndExpense.getAccountId(),incomeAndExpense.getOfficeRunningBalance().abs(),null);
+ n++;
+ }
+ }
+ if(incomeAndExpense.isExpenseAccountType()){
+ if(incomeAndExpense.getOfficeRunningBalance().signum() == 1){
+ creditsJournalEntry[n]= new SingleDebitOrCreditEntryCommand(null,incomeAndExpense.getAccountId(),incomeAndExpense.getOfficeRunningBalance().abs(),null);
+ n++;
+ }else{
+ debitsJournalEntry[m]= new SingleDebitOrCreditEntryCommand(null,incomeAndExpense.getAccountId(),incomeAndExpense.getOfficeRunningBalance().abs(),null);
+ m++;
+ }
+ }
+ }
+ String transactionId = null;
+ if(compare == 1){
+ /* book with target gl id on the credit side */
+ difference = debits.subtract(credits);
+ final SingleDebitOrCreditEntryCommand targetBooking = new SingleDebitOrCreditEntryCommand(null,closureData.getEquityGlAccountId(),difference,null);
+ creditsJournalEntry[n] = targetBooking;
+ journalEntryCommand = new JournalEntryCommand(office.getId(),closureData.getCurrencyCode(),closureData.getClosingDate(),closureData.getComments(),creditsJournalEntry,debitsJournalEntry,closureData.getIncomeAndExpenseComments(),
+ null,null,null,null,null,null,null,null);
+ transactionId = this.journalEntryWritePlatformService.createJournalEntryForIncomeAndExpenseBookOff(journalEntryCommand);
+
+ }else if(compare == -1){
+ /* book with target gl id on the debit side*/
+ difference = credits.subtract(debits);
+ final SingleDebitOrCreditEntryCommand targetBooking = new SingleDebitOrCreditEntryCommand(null,closureData.getEquityGlAccountId(),difference,null);
+ debitsJournalEntry[m]= targetBooking;
+ journalEntryCommand = new JournalEntryCommand(office.getId(),closureData.getCurrencyCode(),closureData.getClosingDate(),closureData.getComments(),creditsJournalEntry,debitsJournalEntry,closureData.getIncomeAndExpenseComments(),
+ null,null,null,null,null,null,null,null);
+ transactionId = this.journalEntryWritePlatformService.createJournalEntryForIncomeAndExpenseBookOff(journalEntryCommand);
+ }else if(compare == 0){
+ //throw new RunningBalanceZeroException(office.getName());
+ return null;
Review comment:
This `return null`here looks very curious... won't this hide problems, are we sure that the (commented out!) throwing of an exception wouldn't be better?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org