fineract-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [fineract] vorburger commented on a change in pull request #796: FINERACT-38
Date Sat, 02 May 2020 18:53:19 GMT

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



Mime
View raw message