[bug] MPaymentTerm.apply(MInovice), trx incorrecta

Inicio Foros Foro principal Desarrolladores [bug] MPaymentTerm.apply(MInovice), trx incorrecta

  • Este debate está vacío.
Viendo 6 entradas - de la 1 a la 6 (de un total de 6)
  • Autor
    Entradas
  • #31781
    Javier Ader
    Participante

    Buenas. No se si es muy importante, pero MPaymentTerm.apply(MInvoice), llama bajo ciertas condiciones a applyNoSchedule(MInvoice); este método llama a deleteInvoicePaySchedule( invoice.getC_Invoice_ID(),get_TrxName() ); esto lo que hace es borrar todos los C_InvoicePaySchedule asociados a la factura. Ok, el tema es que le pasa como parametro la transaccion asociada al termino de pago (a diferencia de applySchedule( MInvoice invoice ), que llama al mismo método pero pasandole la transaccion de la factura, en vez del termino de pago).
    Estas transacciones no siempre son las mismas; en particular, en MInvoice.preparetIt se llama a MInvoice.createPaySchedule(), el cual accede al termino de pago pasandole null como la tranacción.

    Lo correcto pienso que seria o que accediese el termino de pago usando la misma transacción que la factura (en createPaymentSchedule) o que applyNoSchedule utilice la transacción correcta (esto es, la asociada a la Invoice); supongo que esto ultimo es lo correcto.

    En general no deberia generar problemas tal como esta (porque cuando se aplica en schedule, se utiliza la transacción correcta), pero quizá sí desde el TPV; ya que posteriormente a la compleción de la factura se va a acceder (si no me equivoco) a la tabla C_InvoicePaySchedule (el tema es que un delete fuera de una transacción quizá tome locks a nivel de base de datos y ralente las cosas).

    #35738
    Federico Cristina
    Superadministrador

    Javier,

    Muchas gracias por el aporte, lo estaremos revisando a la brevedad.

    Saludos!
    Federico

    #35741
    Matías Nerón Cap
    Superadministrador

    Hola Javier, que tal, por lo visto es verdad lo que comentás, pero lo bueno es que no genera ningún problema. En esta versión de Libertya que está a por salir se fixeó toda la funcionalidad de Esquemas de vencimientos y Esquema de pagos de factura ya que como bien seguro sabés, no estaba andando bien. De todas maneras, eso que indicás me lo anoto para el próximo release.
    En general, no habría problemas ya que la trx null de MPaymentTerm sólo se da en prepareIt (que eso no se porque esta así) y en CalloutInvoice, método paymentTerm, y además el único problema que podría llegar a traer es cuando el esquema de vencimientos es inválido (!isValid()) que es donde entra al applyNoSchedule() para finalmente llamar a deleteInvoicePaySchedule() con la trx incorrecta. La trx de MPaymentTerm se utiliza también al obtener el esquema de pagos.
    En realidad no sé que decirte sobre cuál es la manera correcta de hacerla, si con la trx de la factura o con la trx del propio objeto. De todas maneras la procedencia correcta debería ser que cuando realizamos la aplicación de un esquema de vencimientos sobre una factura, apply(MInvoice), las dos instancias deberían tener las mismas transacciones, no es el caso para cuando se llama a apply(C_Invoice_ID).

    Muchas Gracias por tu aporte…
    Saludos
    Matías Cap

    #35739
    Javier Ader
    Participante

    De nada.
    Matias, que se llame a esta lógica desde MInvoice.prepareIt, no se si esta tan mal, que se lo llame desde el callout, supongo que es para que se regeneren automaticamente cada vez que se cambian y el usuario pueda verlo desde la pestaña “Programa de pagos” o “vencimientos de pagos”, no recuerdo (esto mucho no me gusta, porque indirectamente se modifica a la factura bajo ciertas condiciones; yo preferiría un proceso aparte…).

    Otra cosa que tal vez vi, con la que tengo mi dudas:
    MInvoicePaySchedule en afterSave llaman a MInvoice.ValidatePaySchedule y luego a MInvoice.save()…. eso dentro de la logica anterior no tiene sentido; al menos si los vencimientos de pago se van a generar de manera automática: MPaymentTerm genera los MInvoicePaySchedule (y los salva, lo cual va a llevar al afterSave) , luego llama a MInvoice.validatePaySchedule(); en donde se relee los MInvoicePaySchedule y se verifica que la suma de los mismos es igual al GranTotal; si es asi se los setea como validos y se los vuelve a salvar (ademas de setear MInvoice.isPayScheduleValid…).
    La invocación MInvoice.validatePaySchedule() desde MInvoicePaySchedule no tiene mucho sentido; (ya que MInvoice parece ser el que tiene la facultad de determinar cuando es o no valido un vencimiento) y me da la sensación que solo obedece a una cuestion visual para que en la pestaña pricipipal se actualice el tilde isPayScheduleValid cada vez que se crea un vencimiento en la 4ta solapa…. pero esto ultimo, nuevamente tiene sentido si los vencimientos se van a generar de manera manual; lo cual tiene sentido, por ej, para facturas de Proveedores.

    Pensando en esto, no debería haber un tilde en C_PaymentTerm que diferencie estos dos casos? Digamos: isManual (cuando el termino de pago es manual, el usuario ingresa por si mismo los C_InvoicePaySchedule, poniendo el monto y la fecha de vencimiento de cada pago; toda la logica de generación automatica se saltea si el termino de pago es manual).
    Bajo esta situación MInvoicePaySchedule no llamaria a isPayScheduleValid salvo que pertenezca a un termino de pago manual (aun asi tal vez sea necesario…. no me gusta mucho la idea MInvoice.save() sea llamada desde muchos lados….).

    La idea se completaría así:
    MInvoice.prepareIt:
    -si el termino de pago es automatico, igual que antes (esto sobre el final va terminar seteando isPayScheduleValid)
    -si el termino es manual , se llama validatePaySchedule(); si esta ok (habria que considerar que cuando hay 0 vencimientos, también esta ok; el paySchedule no es valido, pero el usuario no erro los montos de los pagos, simplemente no agrego ninguno), seguimos, si no , hay dos opciones: o setea isPayScheduleValid a falso y se sigue como si nada; o se dispara un error “los montos de los vencimientos no igualan al de la factura”

    En cualquier caso, lo que se hace en el afterSave de MInvoicePaySchedule no seria necesario (el before save pienso que tampoco…)

    Otras temas relacionados:
    – los lookups de termino de pago y de forma de pago deberian modificables cuando la factura esta marcada como pagada (IsPaid; dicho sea de paso este campo no parece ser muy usado, por ej, no se lo tiene en cuenta como filtro para calcular el credito usado una EC, pero parece estar siendo actualizado correctamente)

    – la suma de los montos de los vencimientos, debería ser exactamente igual a la de la factura? Supongo que esto simplifica mucho las cosas, pero no se entiende como se aplicacrian los descuentos en los vencimientos de pago (por ej, de nuevo el calculo del credito usado parece mirar en C_InvoicePaySchedule cuando se valido, es vez de mirar el C_Invoice.GrandTotal; el tema es tal como esta ahora, se va a llegar a los mismo resultados, ya que C_Invoice.GrandTotal = la sumatoria de C_InvoicePaySchedule.DueAmt, si no sería valido el esquema).

    -miraron algún sistema relacionado a ver como están manejando actualmente estas cosas? Despues chusmeo un poco.

    #35745
    Matías Nerón Cap
    Superadministrador

    Si está bien desde el prepareIt, yo me refería del porqué se instancia MPaymentTerm con trx null.
    Como bien decís, creo que toda la funcionalidad de esquema de vencimientos necesita una pulida, sobre todo conceptualmente y a nivel de código, es este primer fixeo dentro de la versión que está a punto de salir se arreglaron muchas de las cuestiones que indicás, pero se dejó mucho código anterior lo cual trae a veces problemas y sobre todas las cosas de performance…un claro ejemplo de código viejo que quedó es el tema del callout, si se va a modificar algo de la factura, no podés salir de la pestaña hasta que no guardas el registro, por lo que sacarlo de ahí no sería nada malo, sobre todo porque la aplicación del esquema de vencimientos también se realiza en el aftersave de la factura, (creo que esto entra para esta versión).
    Con respecto al tema de descuentos/recargos de esquemas de vencimiento te comento que están funcionando en esta versión que va a salir, pero sólo se tienen en cuenta en Recibos de Cliente, al menos por ahora.
    Con respecto a la diferenciación de manual y automático, sinceramente no sé cómo manejan otros sistemas (y al vida real) estos conceptos, no llego a visualizar la utilización de un esquema de vencimientos manual, no debería haber problemas en principio para plasmar un esquema por proveedor. Además, teniendo contacto con el proveedor, mas o menos sabemos cómo es el mecanismo de vencimientos al cual nos está ingresando. De todas maneras, en la pestaña Programa de Pagos de la ventana de facturas, es posible modificar la fecha de vencimiento de la factura propia o de sus cuotas luego de haber sido completada. Cuando está pagada no tiene sentido modificar dicha información.
    Comparto lo mismo sobre MInvoice.save() desde muchos lados, es medio peligroso.
    Bueno, te encargo si querés chusmear cómo se realiza en otros sistemas, pero creo que una solución “prolija” requiere un análisis de muchas situaciones y de los muchos lugares donde se utilizan facturas.
    De todas maneras, me anoto ese fix de la trx incorrecta, quizás lo pueda meter en este, pero creo que ya es medio imposible. Te mantengo al tanto…y GRACIAS por todos los aportes que generás, son de mucha ayuda para nosotros…:)

    Saludos
    Cacho

    #35740
    Javier Ader
    Participante

    Matias, vos decis que la pestaña “de cuotas” (vencimientos de pagos creo que es) es editable? (una pavada pero no lo pude chequiar ya que en la base actual que tengo no tengo ninguna factura con esquemas de pagos…); pense que no, por eso todo el tema de “automatico” vs “manual”. Si es editable (lo cual tiene todo el sentido ahora que lo pienso; esto es, siempre son manuales, al menos respecto a los montos y la fechas) entonces tiene cierto sentido que afterSave de MInvoicePaySchedule (de aca en mas IPS) revalide la factura y la guarde (pero solo bajo ciertas ciertas circunstancias que adelante explico).

    Mire un poco el codigo de Adempiere, y es casi identico, en particular:
    -MInvoice.IsPayScheduleValid es Y si y solo si la sumataria delos IPS.DueAmt es exctamente igual a MInvoice.GrandTotal.
    -IPS.afterSave revalida la factura si se modifico DueAmt (is_ValueChanged(“DueAmt”), lo cual creo que es true incluso en instancias recien creadas); lo caul creo que tambien es un error aca, ya que se hace siempre.
    -la utilización de los porcentajes y descuentos es igual (IPS.DueAmt = GranTotal * Porcentaje; IPS.DiscountAmt = IPS.DueAmt * Porcentaje de descuento )
    -MPaymentTerm.applySchedule : tambien le agrega al ultimo IPS lo que resta si las totales no son exactamente igual (esto es, si uno tiene una 3 cuotas, de 30%, la ultima va ser tratada en la practica como de 40%), y tambien llama sobre el final a MInvoice.validatePaySchedule()
    -Tambien se rendondean incorrectametne los montos de las cuotas (se uiliza la escala de la moneda para realizar la division necesaria para el porcentaje, pero no para redondear el resultado de la mutiplicación; por lo tanto el monto de la cuota va a tener mas decimales que lo que dice la precisión de la moneda….. el tema es que ellos almacenan los montos como numeric, esto es, con cualquier cantidad de decimales; libertya almacena todo con 2 decimales…. esto en libertya se va a producir un redondeo o trucado a nivel de base de datos sobre estos montos, se haga o no el redondeo desde java…)… en cualquier caso, la correccion posterior que hace MPaymentTerm creo que lo soluciona en ambos sistemas (el tema es que ellos pueden llegar a tener el monto de una cuota por ej con valor 33.1234 incluso cuando la precisión de la moneda sea 2; Libetya va a tener necesariamente 33.12, pero esto por que lo hace la base de datos…. )

    Las diferencias:
    -MInvoice cuando vueve a salvar los ips llama a un metodo saveEx, lo cual simplemente es un save() pero que dipsara una excepcion en caso de el save retorne false (esta buena la idea, ya que simplifica mucho el tipico codiogo “if o.save()! return “error o dispara una execpion”. Esto es, el prepareIt falla si falla el guadadado de los IPS (lo cual tieen sentido, si no puede darse el caso de que los IPS queden en estado inconsistente con respecto a MInvoice.IsPayScheduleValid)
    -no se intenta saltear feriados del EC y cosas por el estilo; la fecha del pago es siempre la fecha de facturación mas la cantidad de dias netos datos en C_PaySchedule, lo mismo para la fecha “de descuento” (la cual no se entiende bien si es para un sobrecargo o descuento….)

    En cualquier caso, lo que mas raro veo es el afterSave de los IPS; ya que no tienen en cuenta dos cosas: que la factura ya este paga (en ese caso seria un error, al menos conceptual, creo modificar el plan de pagos), en tal caso deberia disparar un error y si se esta invocando desde MPaymentTerm o MInvoice en estos dos casos es incorrecto la revalidación de la factura y su guardado (esto se va a hacer a posteriormente).
    Dicho esto, pienso que el codigo de MInvoicePaySchedule deberia tener un flag para saltear la revalidación, y que este flag se seteado tanto de MPaymentTerm como Minvoce; algo asi

    Code:
    //MInvoicePaySchedule
    public booelan afterSave(newRedord, sucess)
    {
    if (!sucess) return false;

    if (m_skipInvoiceValidatePaySchedule)
    return true;

    if( is_ValueChanged( “DueAmt” )) {
    log.fine( “afterSave” );
    getParent();
    m_parent.validatePaySchedule();
    m_parent.save(); //IGUAL ESTO ES DUDOSO, tal vez deberia usarse un
    //update directo sobre la factura para setear IsPayScheduleValid
    //con el resultado de validatePaySchedule, asi se saltea toda la logica
    //de after y beforeSave de MInvoice
    }
    return true;
    }

    public beforeSave(newRecord)
    {
    //opcional, agregar esto:
    getParent();
    if (m_parent.isPaid())
    {
    “error la factura ya esta paga”;
    return false;

    }

    //lo que sigue igual
    }

    //finalmente el flag false por defecto; saltea la revalidación y guardado de la factura
    boolean m_skipInvoiceValidatePaySchedule = false;
    public booelan getSkipInvoiceValidatePaySchedule() { return m_skipInvoiceValidatePaySchedule;}
    public void setSkipInvoiceValidatePaySchedule(booelan skip)
    { m_skipInvoiceValidatePaySchedule = skip}

    Finalmente MPayementTerm.applySchedule y MInvoice.validatePaySchedule setean este flag a true en cada instancia de MInvoicePaySchedule a la que acceden (en ambos casos, el que determina si cada IPS y la factura es valida como un todo va a ser MInvoice; no es necesario que cada IPS invoque la revalidación)

    En cuanto a que el AfterSave de Minvoice genere el esquema de pagos (y asi dejar de usar el callout) tiene sentido, pero me parece que trae otros problemas ya que este afterSave se puede llamar muchas veces… Todas estas cosas son necesidades visuales, ya que se van a pisar en el prepareIt…. Tal vez habria que agregar un boton para que el usuario puede regenrar el esquema de pagos cuando lo desee (esto tiene la contra de que se tiene que agregar una columnas mas en la base de datos… tampoco me gusta mucho… habria que encontrar una solucion generar para agregar este tipo de “acciones” a tablas pero sin modificarlas; en vez de esto agregar otras “tablas” de extensión, que generen menus de accciones en la ventanas, especificas para la tabla… hace rato que quiero hacer algo al respecto como una solución general a esta y otras cosas, algun dia tendré tiempo… igual, esto es otro tema)

Viendo 6 entradas - de la 1 a la 6 (de un total de 6)
  • Debes estar registrado para responder a este debate.