Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[14.0][REF] Adapted CNAB implementation after bank.payment.line was removed from account_payment_order module #2386

Merged
merged 21 commits into from
May 28, 2023

Conversation

mbcosta
Copy link
Contributor

@mbcosta mbcosta commented Mar 14, 2023

Adapted CNAB implementation after bank.payment.line was removed from account_payment_order module, but not integrate the use of account.payment object.

Permite atualizar o modulo account_payment_order depois da remoção do objeto bank.payment.line resolvendo o #2296 , mas não integra o uso do account.payment no caso CNAB, as alterações:

  • Métodos e campos que estavam no bank.payment.line foram movidos para o account.payment.line

  • Removido um arquivo não usado e que estava desde a v12 e referencias aos campos generate_move post_move que foram apagados nessa atualização do account_payment_order

  • Modos de Pagamento nos Dados de Demonstração precisam informar o campo edi_format_ids para evitar o erro

psycopg2.IntegrityError: insert or update on table "account_edi_format_account_journal_rel" violates foreign key constraint "account_edi_format_account_journal_rel_account_journal_id_fkey"
DETAIL: Key (account_journal_id)=(33) is not present in table "account_journal".

  • Nos testes é preciso selecionar a Empresa para evitar o erro

odoo.exceptions.UserError: Incompatible companies on records:

  • 'CSH1/2023/03/0001' belongs to company 'TESTE - Simples Nacional' and 'Destination Account' (destination_account_id: '1.1.2.1.01 Clientes') belongs to another company.

  • Inclui a possibilidade de associar todas as account.payment.line relacionadas a um account.move.line com os Eventos de Log CNAB criados e refatorei a visão Formulário desse objeto

image

image

@antoniospneto @felipemotter no modulo l10n_br_cnab_structure além de mover os campos e métodos para o account.payment.line foi preciso fazer as duas alterações abaixo para rodar os testes

  • Erro de permissão de acesso do objeto l10n_br_fiscal.partner.profile, inclui no l10n_br_account a permissão de Leitura para o "Grupo Fatura" ( pelo o que entendo não teria problema esse grupo ter esse acesso ou teria? cc @renatonlima )

    raise AccessError(msg)
    odoo.exceptions.AccessError: You are not allowed to access 'Fiscal Partner Profile' (l10n_br_fiscal.partner.profile) records.

This operation is allowed for the following groups:
- Brazilian Fiscal/Fiscal Manager
- Brazilian Fiscal/Fiscal User

Contact your administrator to request access if necessary.

  • Erro do arquivo CSV eval, segui a lógica de outros campos e alterei de content.strftime para time.strftime

    File "/odoo/external-src/l10n-brazil-REF-update_account_payment_order/l10n_br_cnab_structure/models/cnab_line_field.py", line 202, in output
    value, rec.sending_dynamic_content, **kwargs
    File "/odoo/external-src/l10n-brazil-REF-update_account_payment_order/l10n_br_cnab_structure/models/cnab_line_field.py", line 237, in eval_compute_value
    return safe_eval(python_expression, safe_eval_dict)
    File "/odoo/src/odoo/tools/safe_eval.py", line 347, in safe_eval
    raise ValueError('%s: "%s" while evaluating\n%r' % (ustr(type(e)), ustr(e), expr))
    ValueError: <class 'AttributeError'>: "'str' object has no attribute 'strftime'" while evaluating
    'content.strftime("%d%m%Y")'

Sobre a migração e uso do account.payment no caso CNAB, eu testei criando um banco de dados com os dados de demonstração sem essas alterações Confirmando duas Faturas Unicred e gerando uma Ordem de Debito até o fim e fazendo o retorno do CNAB de um caso e deixando a outra Ordem no estado Confirmado.

Caso sejam criados os account.payment, o que é feito pelo account_payment_order, apesar de existirem somente dois account.move o script de migração acaba criando vários account.payment o que fica errado, pois só deveriam existir dois Pagamentos
image

Existem outros problemas relacionados a implementação do account_payment_order o código considera que toda account.payment.line deve gerar um account.payment o que não é o caso do CNAB devido a possibilidade de mandar outras Instruções como "Alteração de Vencimento", "Desconto" e etc ( https://github.com/OCA/l10n-brazil/blob/14.0/l10n_br_account_payment_order/models/l10n_br_cnab_change_methods.py ), onde nesses casos não deve criar um account.payment e ao informar que o arquivo foi Enviado/File Uploaded ocorre uma Reconciliação dos account.payment criados o que também não é real no caso CNAB Cobrança porque o banco precisa antes Confirmar a Instrução e a reconciliação só deve ocorrer quando o Banco informa via CNAB que houve esse pagamento, o código não está considerando que existem recebimentos a prazo.

Por isso optei por sobre escrever o método draft2open do account.payment.order para que nos casos CNAB não seja criado o account.payment mantendo assim o mesmo comportamento que existe hoje e dessa forma ser possível atualizar o account_payment_order e deixar tanto o uso do account.payment quanto do modulo https://github.com/OCA/bank-payment/tree/14.0/account_payment_order_return para tratar o Arquivo de Retorno do CNAB comentados no RFC #2272 no ROADMAP do modulo e dessa forma também ter PRs específicos para essas mudanças.

Por isso o script de migração do l10n_br_account_payment_order está apagando os account.payment criados pela migração do account_payment_order( é necessário confirmar se isso também é válido para o caso do l10n_br_cnab_structure ) e também está relacionando os Eventos de Log do CNAB com as Payment Lines.

Existem um problema no ORM relacionado a apagar um objeto que tenha campos do tipo Selection

2023-03-10 21:15:22,323 72 INFO db odoo.addons.base.models.ir_model: Deleting [email protected] (l10n_br_account_payment_order.selection__bank_payment_line__last_cnab_state__done)
2023-03-10 21:15:22,326 72 WARNING db odoo.modules.loading: Transient module states were reset
2023-03-10 21:15:22,328 72 ERROR db odoo.modules.registry: Failed to load registry
2023-03-10 21:15:22,328 72 CRITICAL db odoo.service.server: Failed to initialize database db.
Traceback (most recent call last):
File "/odoo/src/odoo/service/server.py", line 1201, in preload_registries
registry = Registry.new(dbname, update_module=update_module)
File "/odoo/src/odoo/modules/registry.py", line 89, in new
odoo.modules.load_modules(registry._db, force_demo, status, update_module)
File "/odoo/src/odoo/modules/loading.py", line 505, in load_modules
env['ir.model.data']._process_end(processed_modules)
File "/odoo/src/odoo/addons/base/models/ir_model.py", line 2310, in _process_end
self._process_end_unlink_record(record)
File "/odoo/src/odoo/addons/base/models/ir_model.py", line 2233, in _process_end_unlink_record
record.unlink()
File "/odoo/src/odoo/addons/base/models/ir_model.py", line 1380, in unlink
self._process_ondelete()
File "/odoo/src/odoo/addons/base/models/ir_model.py", line 1418, in _process_ondelete
Model = self.env[selection.field_id.model]
File "/odoo/src/odoo/api.py", line 476, in getitem
return self.registry[model_name]._browse(self, (), ())
File "/odoo/src/odoo/modules/registry.py", line 177, in getitem
return self.models[model_name]
KeyError: 'bank.payment.line'

Devido a isso foi necessário recriar o objeto bank.payment.line mesmo vazio para evitar o erro, referencia do problema e solução odoo/odoo#44767

Dessa forma ao rodar o update o erro não acontece e os campos são apagados sem erro, e depois do primeiro update o objeto pode ser apagado e um terceiro update limpa o(s) Warning que restaram, esse processo pode ser testado dessa forma, porém para evitar problemas recomendo a remoção do objeto na migração de versão v15 ou v16, e assim vai poder ser feito de forma segura.

IMPORTANTE: Não atualize um ambiente e um banco de dados de produção diretamente, crie um ambiente de testes e valide as alterações rodando um
$ odoo -d NOME_DO_BD -u account_payment_order --workers 0 --stop-after-init
Verifique o LOG e veja se teve algum erro, se ocorrer informe o erro aqui e dependendo do problema, se não for uma customização, podemos ver de adaptar o script de migração.

cc @rvalyi @marcelsavegnago @mileo

@OCA-git-bot
Copy link
Contributor

Hi @felipemotter, @rvalyi, @antoniospneto, @renatonlima,
some modules you are maintaining are being modified, check this out!

@antoniospneto
Copy link
Contributor

Legal @mbcosta pretendemos estudar a PR em breve, essa nova mudança realmente tem um impacto bem grande , por isso vamos separar um tempo para poder testar bem , valeu !

@marcelsavegnago
Copy link
Member

marcelsavegnago commented Mar 20, 2023

Pode ser algo na base que estou usando mas na atualizacao o sistema está apresentando mensagem de erro sobre account_move sem fiscal_document_id. São 11 registros e todos com a referencia PAYXXXXX

image

image

@marcelsavegnago
Copy link
Member

marcelsavegnago commented Mar 20, 2023

Dei mais uma olhada e os unicos moves sem fiscal document são relacionados a estes pagamentos que estao no status Arquivo Gerado apenas.

image

image

Tem pelo menos uns 4 registros com essa referencia.. e todos sem nenhuma linha.
image

@marcelsavegnago
Copy link
Member

Aqui o select no account_move_line

image

@mbcosta
Copy link
Contributor Author

mbcosta commented Mar 31, 2023

Obrigado @marcelsavegnago pela revisão realmente faltava o script de migração apagar as account.move.line e account.move ligadas aos account.payment criados pelo openupgrade do account_payment_order, atualizei o script para fazer isso, agradeço se puder testar novamente.

@marcelsavegnago
Copy link
Member

Obrigado @marcelsavegnago pela revisão realmente faltava o script de migração apagar as account.move.line e account.move ligadas aos account.payment criados pelo openupgrade do account_payment_order, atualizei o script para fazer isso, agradeço se puder testar novamente.

Vou testar sim @mbcosta .. parabéns pelo trabalho.

@marcelsavegnago
Copy link
Member

Obrigado @marcelsavegnago pela revisão realmente faltava o script de migração apagar as account.move.line e account.move ligadas aos account.payment criados pelo openupgrade do account_payment_order, atualizei o script para fazer isso, agradeço se puder testar novamente.

@mbcosta fiz uma nova atualização e agora e o problema relatado anteriomente nao ocorreu. Vlwww.

Copy link
Member

@marcelsavegnago marcelsavegnago left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Atualizei uma base considerando esta adaptação e aparentemente está ok. Não me aprofundei na revisão do código e nem nos testes funcionais mas me parece ok.

@mbcosta
Copy link
Contributor Author

mbcosta commented Apr 20, 2023

valeu @marcelsavegnago pela revisão, se tiver dúvidas sobre o código ou testes é só perguntar, o funcionamento do modulo permanece o mesmo, você pode testar com os dados de demonstração Confirmando uma das Faturas do caso Uncred 400 será criada automaticamente uma Ordem de Débito com as duas linhas e ao Confirmar vai ser possível Gerar o TXT referente a esse CNAB, a partir disso se quiser é possível testar também uma alteração exemplo Alterar uma Data de Vencimento que vai gerar outra Ordem de Debito, e depois você pode testar o CNAB de Retorno enviado pelo Banco, os aquivos estão na pasta de testes do modulo l10n_br_account_payment_brcobranca https://github.com/OCA/l10n-brazil/tree/14.0/l10n_br_account_payment_brcobranca/tests/data o arquivo CNAB400UNICRED_valor_menor_1.RET simula apenas a Confirmação e o CNAB400UNICRED_valor_menor_2.RET é o que possui a Liquidação e deve Reconciliar os lançamentos e deixar a Fatura como Paga, existem dois casos do Unicred 400 porque um, esse primeiro caso, simula quando Valor Recebido é Menor do que a Fatura e o segundo quando é Maior; os testes quando rodam localmente também simulam e testam esse e outros processos.

@marcelsavegnago
Copy link
Member

marcelsavegnago commented Apr 24, 2023

Vlww @mbcosta. Em breve o pessoal deve conseguir revisar a PR também e ai destravamos este assunto.

cc @antoniospneto @douglascstd @augustodinizl @felipemotter

@mbcosta mbcosta force-pushed the 14.0-REF-update_account_payment_order branch from 8a0cc08 to 424d3e9 Compare April 26, 2023 18:12
@marcelsavegnago
Copy link
Member

marcelsavegnago commented May 24, 2023

Bom dia pessoal!

Seria ótimo conseguir destravar essa Pull Request (PR). O que vocês acham?

@rvalyi
Copy link
Member

rvalyi commented May 24, 2023

hehe @marcelsavegnago eu ia falar a mesma coisa hoje tb. Do meu lado vou testar ainda hoje. Eu tb diria que ja demos bastante tempo para as pessoas correr atras de seguir o refator. Vou testar e retornar hoje do meu lado.

mbcosta and others added 18 commits May 27, 2023 22:51
…e was removed in account_payment_order module.
… because it just appear when specific CNAB dont't have custom fields.
…ayment.mode onchange methods and removed unexist fields.
…e with multiple CNAB Log Events and refactoring CNAB Log Event Form view.
…h CNAB Log Events even when it's not a Liquity Instruction.
…_order module after removed bank.payment.line object.
@rvalyi rvalyi force-pushed the 14.0-REF-update_account_payment_order branch from e1c40f3 to 2a4de57 Compare May 28, 2023 02:57
Copy link
Member

@rvalyi rvalyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parabens pelo trabalho pesado @mbcosta ! O refator me parece correto. Esperamos o bastante para pessoas testarem, agora a gente precisa andar com esse merge para tirar a divida tecnológica que representa a versão defasada do account_payment_order.
Eu limpei alguns __init__.py e ajustei o numero das versões para refletir as mudanças major ou minor.

@rvalyi
Copy link
Member

rvalyi commented May 28, 2023

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 14.0-ocabot-merge-pr-2386-by-rvalyi-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 3c4cf16 into OCA:14.0 May 28, 2023
@OCA-git-bot
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants