[14.0][FIX] l10n_br_account_payment_order: tracking parameter

[marcelsavegnago]

No description provided.

[marcelsavegnago]

@mbcosta estou fazendo um teste aqui incluindo a dependencia

[marcelsavegnago]

@mbcosta agora parece que foi.. se puder dar uma olhada eu agradeço.

cc @renatonlima @rvalyi @mileo

[marcelsavegnago]

@mbcosta ficou faltando o inverse_name.

[rvalyi]

@mbcosta @marcelsavegnago a gente já não tinha removido esses mail.thread desses objetos para não criar problemas de performance na 12?? Me parece que sim.
Vejam aqui como essa ideia foi recebida no modulo account_payment_order quando @mileo tinha ido propor isso la:
OCA/bank-payment#781

Sera se não faltou remover os keywords de tracking quando a gente tirou esses inherit do mail.thread?

[marcelsavegnago]

@mbcosta @marcelsavegnago a gente já não tinha removido esses mail.thread desses objetos para não criar problemas de performance na 12?? Me parece que sim. Vejam aqui como essa ideia foi recebida no modulo account_payment_order quando @mileo tinha ido propor isso la: OCA/bank-payment#781

Sera se não faltou remover os keywords de tracking quando a gente tirou esses inherit do mail.thread?

@rvalyi pode ser que faltou remover o tracking dos campos. vou dar uma olahda na proposta do Mileo

[rvalyi]

@mbcosta @marcelsavegnago a gente já não tinha removido esses mail.thread desses objetos para não criar problemas de performance na 12?? Me parece que sim. Vejam aqui como essa ideia foi recebida no modulo account_payment_order quando @mileo tinha ido propor isso la: OCA/bank-payment#781
Sera se não faltou remover os keywords de tracking quando a gente tirou esses inherit do mail.thread?

@rvalyi pode ser que faltou remover o tracking dos campos. vou dar uma olahda na proposta do Mileo

não tem nada para usar de la, mas era apenas para você ver toda galera experiente falando que era zoado de querer enfiar o mail.thread em objetos utilizados de forma intensiva como esses onde vc adicionou nessa PR. Tou achando que o melhor para resolver os warning é so tirar esses tracking=. mail.thread é algo que tem um custo de performance alto, não é para usar em qualquer lugar, muito menos dentro de lignas de de registros.

[marcelsavegnago]

@mbcosta @marcelsavegnago a gente já não tinha removido esses mail.thread desses objetos para não criar problemas de performance na 12?? Me parece que sim. Vejam aqui como essa ideia foi recebida no modulo account_payment_order quando @mileo tinha ido propor isso la: OCA/bank-payment#781
Sera se não faltou remover os keywords de tracking quando a gente tirou esses inherit do mail.thread?

@rvalyi pode ser que faltou remover o tracking dos campos. vou dar uma olahda na proposta do Mileo

não tem nada para usar de la, mas era apenas para você ver toda galera experiente falando que era zoado de querer enfiar o mail.thread em objetos utilizados de forma intensiva como esses onde vc adicionou nessa PR. Tou achando que o melhor para resolver os warning é so tirar esses tracking=. mail.thread é algo que tem um custo de performance alto, não é para usar em qualquer lugar, muito menos dentro de lignas de de registros.

Show.. por mim ok.. farei isso

[marcelsavegnago]

@rvalyi removi o tracking que havia dentro do account.payment.line. Porém, as demais alterações eu mantive.. acha valido remover o tracking de todos os campos deste módulo ?

[mbcosta]

@rvalyi @marcelsavegnago no PR que foi feito no modulo account_payment_order foi recusada a proposta de adicionar o mail.thread no objeto account.move.line, o que eu também não vejo necessidade porque se podia usar o mail.thread do antigo account.invoice hoje o account.move. Nesse caso o objeto que foi incluído é o account.payment.mode que não é um objeto alterado ou criado com frequência ( por isso não teria problema com performance ) mas que é necessário usar o Tracking em determinados campos por ser preciso ter o registro do histórico de alterações, qualquer modificação nos campos referentes ao CNAB podem causar diversos problemas desde de parâmetros incorretos no tempo e valor de cobrança de Multa até mesmo na reconciliação de Faturas pagas, e em caso de problemas tanto o usuário quanto o desenvolvedor responsável podem rapidamente validar se houve alterações nesses campos.

O erro que está acontecendo aqui na minha opinião pode ser debatido até do ponto de vista da API ORM, porque esse é o caso de um objeto que é herdado e dentro do outro objeto é que se deseja usar o mail.thread. Nesse caso do l10n_br_account_payment_order basicamente foi criada uma classe para ter os campos do Boleto em separado https://github.com/OCA/l10n-brazil/blob/14.0/l10n_br_account_payment_order/models/l10n_br_cnab_boleto_fields.py e no objeto account.payment.mode essa classe é herdada https://github.com/OCA/l10n-brazil/blob/14.0/l10n_br_account_payment_order/models/account_payment_mode.py#L19, a ideia era facilitar a manutenção por serem muitos campos, e essa classe dos campos do Boleto não deveria ter o mail.thread e sim a account.payment.mode onde realmente isso é usado, mas houve alteração no uso desse parâmetro na v14 e é preciso identificar a melhor forma de fazer esse sistema de heranças do mail.thread entre esses objetos para remover os Warnings, eu comentei no PR #1888 de migração do l10n_br_account_payment_order :

"""
sobre esse erro eu estava tentando entender o porque de estar gerando os Warnings; esse objeto dos campos do Boleto é herdado no accoun.payment.mode e lá existe a herança do mail.thread e isso funcionava e funciona na v14 mesmo com os Warnings, procurei resolver testando a inclusão do "mail.thread", "mail.activity.mixin" mas gera erro:

File "/odoo/src/odoo/models.py", line 585, in _build_model
ModelClass.bases = tuple(bases)
TypeError: Cannot create a consistent method resolution
order (MRO) for bases Model, mail.thread, mail.activity.mixin, l10n_br_cnab.boleto.fields, l10n_br_cnab.payment.fields, base

É preciso, além de incluir no objeto do campos do Boleto essa herança, remover a herança do mail.thread no account.payment.mode para funcionar, fica estranho o sistema de heranças, mas até onde testei também funciona e deixa de aparecer os Warnings ( talvez exista uma outra forma disso ser feito ).
"""

Olhando hoje uma forma de evitar essa questão de colocar o mail.thread no objeto dos campos do boleto são:

• Remover a classe dos Boletos e mover todos os campos diretamente para o account.payment.mode
• Ou remover o paramento tracking na classe dos Boletos e na classe account.payment.mode sobre escrever os campos dos Boletos que tem a necessidade de usar o tracking

Eu prefiro a segunda opção por manter a maior parte dos campos que não tem o Tracking em uma classe em separado, deixando mais "limpo" o arquivo do account.payment.mode.

Portando eu sou contra remover o Tracking e consequentemente o mail.thread no account.payment.mode por questão do registro de alterações, no caso do objeto account.payment.line sou a favor, pelo o que vi apenas o campo payment_mode_id está com o tracking https://github.com/OCA/l10n-brazil/blob/14.0/l10n_br_account_payment_order/models/account_payment_line.py#L92 mas esse campo por enquanto é sempre igual ao Modo de Pagamento principal na Ordem de Debito/account.payment.order e provavelmente foi incluído pensando no caso de "Múltiplos Modos de Pagamento em uma mesma Ordem de Debito/Pagto" odoo-brazil#112 mas hoje o campo pode ser deixado como Readonly na visão para evitar alteração e a necessidade de ser monitorado.

[marcelsavegnago]

Eu prefiro a segunda opção por manter a maior parte dos campos que não tem o Tracking em uma classe em separado, deixando mais "limpo" o arquivo do account.payment.mode.

Também dou a favor da segunda opção. Por ora, das alterações que eu havia enviado, acertei a questão do account.payment.line conforme comentado por você e pelo @rvalyi

[rvalyi]

Ola @marcelsavegnago valeu pela limpeza, porem revisando eu vi um problema que eu aproveitei para corrigir aqui Escodoo#47

cc @mbcosta @renatonlima @netosjb

[marcelsavegnago]

Ola @marcelsavegnago valeu pela limpeza, porem revisando eu vi um problema que eu aproveitei para corrigir aqui Escodoo#47

cc @mbcosta @renatonlima @netosjb

Done

[mbcosta]

valeu @rvalyi não imaginava que o problema poderia ser resolvido alterando o tipo da classe.

@marcelsavegnago restaram apenas dois Warnings https://github.com/OCA/l10n-brazil/actions/runs/3099407701/jobs/5018509628#step:8:724 :

• WARNING odoo odoo.fields: Field account.move.cnab_return_log_id: unknown parameter 'inverse_name', if this is an actual parameter you may want to override the method _valid_field_parameter on the relevant model in order to allow it

Esse parece ser um problema da migração, na v12 para cada linha de recebimento do Retorno do CNAB é criado um Lançamento/account.move por isso a ligação com Log de Retorno, mas com a junção do account.move com o account.invoice isso parece ter ficado errado, mas acredito que podemos deixar isso para outro PR

• WARNING odoo odoo.fields: Field account.payment.mode.code_convenio_lider: unknown parameter 'track_visibility', if this is an actual parameter you may want to override the method _valid_field_parameter on the relevant model in order to allow it

Esse pode ser corrigido acredito que foi feito um cherry-pick da v12 onde o parametro é track_visibility="always", mas na v14 é tracking=True, aqui https://github.com/OCA/l10n-brazil/blob/14.0/l10n_br_account_payment_order/models/l10n_br_cnab_boleto_fields.py#L31

[OCA-git-bot]

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

[rvalyi]

bom, já vou fazer o merge desse para começar e depois a gente vê dos outros pontos...

/ocabot merge minor

[OCA-git-bot]

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 14.0-ocabot-merge-pr-1953-by-rvalyi-bump-minor, awaiting test results.

[OCA-git-bot]

Congratulations, your PR was merged at 327c0da. Thanks a lot for contributing to OCA. ❤️