details: https://code.openbravo.com/erp/devel/pi/rev/ee738640c404 changeset: 29824:ee738640c404 user: Mark <markmm82 <at> gmail.com> date: Thu Aug 04 09:52:25 2016 -0400 summary: Fixes issue 33550: Error voiding an invoice if bp doesn't have default account
Method used to get the default financial account (getFinancialAccountPaymentMethod) was not taking into account the natural tree of the organization of document being processed, always was ordering by "default" account and when the query returns more than one results, if the first record's account does not belong to the natural tree of the entity's organization (in this case invoice's organization) validation was failing and an error message was shown. To avoid it, was overwritten the getFinancialAccountPaymentMethod method of the FIN_Utility class to also filtering by financial accounts belonging to the natural tree of the entity's organization. Was necessary to adapt other classes using this method to work correctly. details: https://code.openbravo.com/erp/devel/pi/rev/ae1076c7e9ea changeset: 29825:ae1076c7e9ea user: Alvaro Ferraz <alvaro.ferraz <at> openbravo.com> date: Thu Aug 04 16:07:56 2016 +0200 summary: Related to issue 33550: Code review improvements Remove unused OrganizationStructureProvider. Remove duplicated method. Remove orgId1 parameter as it can be calculated inside the query. diffstat: modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/ad_actionbutton/ProcessInvoice.java | 18 +--- modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/filterexpression/AddPaymentDefaultValuesHandler.java | 37 ++------ modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/utility/FIN_Utility.java | 41 ++++++--- 3 files changed, 41 insertions(+), 55 deletions(-) diffs (239 lines): diff -r 57f68b22dd06 -r ae1076c7e9ea modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/ad_actionbutton/ProcessInvoice.java --- a/modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/ad_actionbutton/ProcessInvoice.java Fri Aug 05 09:14:45 2016 +0200 +++ b/modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/ad_actionbutton/ProcessInvoice.java Thu Aug 04 16:07:56 2016 +0200 @@ -49,7 +49,6 @@ import org.openbravo.base.secureApp.VariablesSecureApp; import org.openbravo.base.session.OBPropertiesProvider; import org.openbravo.dal.core.OBContext; -import org.openbravo.dal.security.OrganizationStructureProvider; import org.openbravo.dal.service.OBCriteria; import org.openbravo.dal.service.OBDal; import org.openbravo.dal.service.OBDao; @@ -218,8 +217,6 @@ isSOTrx ? AcctServer.DOCTYPE_ARReceipt : AcctServer.DOCTYPE_APPayment); final String strPaymentDocumentNo = FIN_Utility.getDocumentNo(docType, docType.getTable() != null ? docType.getTable().getDBTableName() : ""); - final OrganizationStructureProvider osp = OBContext.getOBContext() - .getOrganizationStructureProvider(invoice.getClient().getId()); // Get default Financial Account as it is done in Add Payment FIN_FinancialAccount bpFinAccount = null; @@ -227,24 +224,19 @@ && invoice.getBusinessPartner().getAccount() != null && FIN_Utility.getFinancialAccountPaymentMethod(invoice.getPaymentMethod().getId(), invoice.getBusinessPartner().getAccount().getId(), isSOTrx, invoice - .getCurrency().getId()) != null - && osp.isInNaturalTree(invoice.getBusinessPartner().getAccount().getOrganization(), - invoice.getOrganization())) { + .getCurrency().getId(), invoice.getOrganization().getId()) != null) { bpFinAccount = invoice.getBusinessPartner().getAccount(); } else if (!isSOTrx && invoice.getBusinessPartner().getPOFinancialAccount() != null && FIN_Utility.getFinancialAccountPaymentMethod(invoice.getPaymentMethod().getId(), invoice.getBusinessPartner().getPOFinancialAccount().getId(), isSOTrx, invoice - .getCurrency().getId()) != null - && osp.isInNaturalTree(invoice.getBusinessPartner().getPOFinancialAccount() - .getOrganization(), invoice.getOrganization())) { + .getCurrency().getId(), invoice.getOrganization().getId()) != null) { bpFinAccount = invoice.getBusinessPartner().getPOFinancialAccount(); } else { FinAccPaymentMethod fpm = FIN_Utility.getFinancialAccountPaymentMethod(invoice - .getPaymentMethod().getId(), null, isSOTrx, invoice.getCurrency().getId()); - if (fpm != null - && osp.isInNaturalTree(fpm.getAccount().getOrganization(), - invoice.getOrganization())) { + .getPaymentMethod().getId(), null, isSOTrx, invoice.getCurrency().getId(), + invoice.getOrganization().getId()); + if (fpm != null) { bpFinAccount = fpm.getAccount(); } } diff -r 57f68b22dd06 -r ae1076c7e9ea modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/filterexpression/AddPaymentDefaultValuesHandler.java --- a/modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/filterexpression/AddPaymentDefaultValuesHandler.java Fri Aug 05 09:14:45 2016 +0200 +++ b/modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/filterexpression/AddPaymentDefaultValuesHandler.java Thu Aug 04 16:07:56 2016 +0200 @@ -11,7 +11,7 @@ * under the License. * The Original Code is Openbravo ERP. * The Initial Developer of the Original Code is Openbravo SLU - * All portions are Copyright (C) 2014-2015 Openbravo SLU + * All portions are Copyright (C) 2014-2016 Openbravo SLU * All Rights Reserved. * Contributor(s): ______________________________________. ************************************************************************ @@ -29,8 +29,6 @@ import org.codehaus.jettison.json.JSONObject; import org.openbravo.advpaymentmngt.dao.AdvPaymentMngtDao; import org.openbravo.advpaymentmngt.utility.FIN_Utility; -import org.openbravo.dal.core.OBContext; -import org.openbravo.dal.security.OrganizationStructureProvider; import org.openbravo.dal.service.OBDal; import org.openbravo.model.common.businesspartner.BusinessPartner; import org.openbravo.model.common.enterprise.Organization; @@ -290,8 +288,6 @@ if (StringUtils.isNotEmpty(paymentMethodId) && context.has("inpadClientId") && context.has("inpadOrgId")) { - final OrganizationStructureProvider osp = OBContext.getOBContext() - .getOrganizationStructureProvider(context.getString("inpadClientId")); String strBPartnerId = getDefaultReceivedFrom(requestMap); boolean isSOTrx = "Y".equals(getDefaultIsSOTrx(requestMap)); if (StringUtils.isNotEmpty(strBPartnerId)) { @@ -301,25 +297,20 @@ if (isSOTrx && businessPartner.getAccount() != null && FIN_Utility.getFinancialAccountPaymentMethod(paymentMethodId, businessPartner - .getAccount().getId(), isSOTrx, currencyId) != null - && osp.isInNaturalTree(businessPartner.getAccount().getOrganization(), OBDal - .getInstance().get(Organization.class, context.getString("inpadOrgId")))) { + .getAccount().getId(), isSOTrx, currencyId, context.getString("inpadOrgId")) != null) { return businessPartner.getAccount().getId(); } else if (!isSOTrx && businessPartner.getPOFinancialAccount() != null && FIN_Utility.getFinancialAccountPaymentMethod(paymentMethodId, businessPartner - .getPOFinancialAccount().getId(), isSOTrx, currencyId) != null - && osp.isInNaturalTree(businessPartner.getPOFinancialAccount().getOrganization(), OBDal - .getInstance().get(Organization.class, context.getString("inpadOrgId")))) { + .getPOFinancialAccount().getId(), isSOTrx, currencyId, context + .getString("inpadOrgId")) != null) { return businessPartner.getPOFinancialAccount().getId(); } } FinAccPaymentMethod fpm = FIN_Utility.getFinancialAccountPaymentMethod(paymentMethodId, null, - isSOTrx, currencyId); - if (fpm != null - && osp.isInNaturalTree(fpm.getAccount().getOrganization(), - OBDal.getInstance().get(Organization.class, context.getString("inpadOrgId")))) { + isSOTrx, currencyId, context.getString("inpadOrgId")); + if (fpm != null) { return fpm.getAccount().getId(); } } @@ -369,7 +360,7 @@ strFinancialAccountId = getContextFinancialAccount(requestMap); if (strFinPaymentMethodId != null && FIN_Utility.getFinancialAccountPaymentMethod(strFinPaymentMethodId, - strFinancialAccountId, isSOTrx, null) != null) { + strFinancialAccountId, isSOTrx, null, context.getString("inpadOrgId")) != null) { return strFinPaymentMethodId; } @@ -377,26 +368,18 @@ if (StringUtils.isNotEmpty(strBPartnerId) && context.has("inpadClientId") && context.has("inpadOrgId")) { - final OrganizationStructureProvider osp = OBContext.getOBContext() - .getOrganizationStructureProvider(context.getString("inpadClientId")); BusinessPartner businessPartner = OBDal.getInstance().get(BusinessPartner.class, strBPartnerId); if (isSOTrx && businessPartner.getPaymentMethod() != null && FIN_Utility.getFinancialAccountPaymentMethod(businessPartner.getPaymentMethod() - .getId(), strFinancialAccountId, isSOTrx, null) != null - && osp.isInNaturalTree(businessPartner.getPaymentMethod().getOrganization(), OBDal - .getInstance().get(Organization.class, context.getString("inpadOrgId")))) { + .getId(), strFinancialAccountId, isSOTrx, null, context.getString("inpadOrgId")) != null) { return businessPartner.getPaymentMethod().getId(); - } - - else if (!isSOTrx + } else if (!isSOTrx && businessPartner.getPOPaymentMethod() != null && FIN_Utility.getFinancialAccountPaymentMethod(businessPartner.getPOPaymentMethod() - .getId(), strFinancialAccountId, isSOTrx, null) != null - && osp.isInNaturalTree(businessPartner.getPOPaymentMethod().getOrganization(), OBDal - .getInstance().get(Organization.class, context.getString("inpadOrgId")))) { + .getId(), strFinancialAccountId, isSOTrx, null, context.getString("inpadOrgId")) != null) { return businessPartner.getPOPaymentMethod().getId(); } } diff -r 57f68b22dd06 -r ae1076c7e9ea modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/utility/FIN_Utility.java --- a/modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/utility/FIN_Utility.java Fri Aug 05 09:14:45 2016 +0200 +++ b/modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/utility/FIN_Utility.java Thu Aug 04 16:07:56 2016 +0200 @@ -1084,8 +1084,7 @@ invoiceDocNo = invoice.getDocumentNo(); final String paymentDescription = OBDal.getInstance() - .get(OrganizationInformation.class, (organization.getId())) - .getAPRMPaymentDescription(); + .get(OrganizationInformation.class, (organization.getId())).getAPRMPaymentDescription(); // In case of a purchase invoice and the Supplier Reference is selected use // Reference if (paymentDescription.equals("Supplier Reference") && !invoice.isSalesTransaction()) { @@ -1590,6 +1589,12 @@ updateCustomerCredit(businessPartner, amount, false); } + public static FinAccPaymentMethod getFinancialAccountPaymentMethod(String paymentMethodId, + String financialAccountId, boolean issotrx, String currencyId) { + return getFinancialAccountPaymentMethod(paymentMethodId, financialAccountId, issotrx, + currencyId, null); + } + /** * Get an active FinAccPaymentMethod related to paymentMethodId FIN_PaymentMethod and * financialAccountId FIN_FinancialAccount, if exists. If paymentMethodId is null it will retrieve @@ -1598,28 +1603,33 @@ * if currencyId is not null. */ public static FinAccPaymentMethod getFinancialAccountPaymentMethod(String paymentMethodId, - String financialAccountId, boolean issotrx, String currencyId) { + String financialAccountId, boolean issotrx, String currencyId, String orgId) { StringBuffer where = new StringBuffer(); where.append(" as fapm"); where.append(" join fapm." + FinAccPaymentMethod.PROPERTY_ACCOUNT + " as fa"); - where.append(" where fapm." + FinAccPaymentMethod.PROPERTY_PAYMENTMETHOD + " = :paymentMethod"); + where.append(" where fapm." + FinAccPaymentMethod.PROPERTY_PAYMENTMETHOD + + ".id = :paymentMethodId"); where.append(" and fa." + FIN_FinancialAccount.PROPERTY_ACTIVE + " = true"); if (issotrx) { where.append(" and fapm." + FinAccPaymentMethod.PROPERTY_PAYINALLOW + " = true"); } else { where.append(" and fapm." + FinAccPaymentMethod.PROPERTY_PAYOUTALLOW + " = true"); } - if (!StringUtils.isEmpty(financialAccountId)) { - where.append(" and fapm." + FinAccPaymentMethod.PROPERTY_ACCOUNT + " = :financialAccount"); + if (StringUtils.isNotEmpty(financialAccountId)) { + where.append(" and fa." + FIN_FinancialAccount.PROPERTY_ID + " = :financialAccountId"); } - if (!StringUtils.isEmpty(currencyId)) { - where.append(" and (fa." + FIN_FinancialAccount.PROPERTY_CURRENCY + " = :currency"); + if (StringUtils.isNotEmpty(currencyId)) { + where.append(" and (fa." + FIN_FinancialAccount.PROPERTY_CURRENCY + ".id = :currencyId"); if (issotrx) { where.append(" or fapm." + FinAccPaymentMethod.PROPERTY_PAYINISMULTICURRENCY + " = true)"); } else { where.append(" or fapm." + FinAccPaymentMethod.PROPERTY_PAYOUTISMULTICURRENCY + " = true)"); } } + if (StringUtils.isNotEmpty(orgId)) { + where.append(" and ad_org_isinnaturaltree(fa." + FIN_FinancialAccount.PROPERTY_ORGANIZATION + + ".id, :orgId, fa." + FIN_FinancialAccount.PROPERTY_CLIENT + ".id) = 'Y'"); + } where.append(" order by fapm." + FinAccPaymentMethod.PROPERTY_DEFAULT + " desc"); OBQuery<FinAccPaymentMethod> qry = OBDal.getInstance().createQuery(FinAccPaymentMethod.class, @@ -1627,14 +1637,15 @@ qry.setFilterOnReadableOrganization(false); qry.setMaxResult(1); - qry.setNamedParameter("paymentMethod", - OBDal.getInstance().get(FIN_PaymentMethod.class, paymentMethodId)); - if (!StringUtils.isEmpty(financialAccountId)) { - qry.setNamedParameter("financialAccount", - OBDal.getInstance().get(FIN_FinancialAccount.class, financialAccountId)); + qry.setNamedParameter("paymentMethodId", paymentMethodId); + if (StringUtils.isNotEmpty(financialAccountId)) { + qry.setNamedParameter("financialAccountId", financialAccountId); } - if (!StringUtils.isEmpty(currencyId)) { - qry.setNamedParameter("currency", OBDal.getInstance().get(Currency.class, currencyId)); + if (StringUtils.isNotEmpty(currencyId)) { + qry.setNamedParameter("currencyId", currencyId); + } + if (StringUtils.isNotEmpty(orgId)) { + qry.setNamedParameter("orgId", orgId); } return (FinAccPaymentMethod) qry.uniqueResult(); ------------------------------------------------------------------------------ _______________________________________________ Openbravo-commits mailing list Openbravo-commits@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openbravo-commits
