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+IMP] l10n_br_account_payment_order, l10n_br_account_payment_brcobranca: Adaptando o código para atender o Santander 240 e 400 #2925

Merged

Conversation

mbcosta
Copy link
Contributor

@mbcosta mbcosta commented Feb 24, 2024

Adapting cnab santander 240 400.

Adaptando o código para atender o Santander 240 e 400, esse PR é uma refatoração referente ao que foi feito na v12 #2012 o port na v14 #2848 e o #2870 ( esse PR deve substituir o último ), ao ver esse caso acabou sendo necessário fazer algumas refatorações tanto no código para evitar a duplicação de campos, porque o CNAB não tem padrão mesmo na Nomenclatura, quanto outras melhorias, segue:

  • Permitir adicionar Comentários/Observações nos Códigos de Retorno do CNAB e com isso inclui informações no caso Santander 240 e 400, também refatorei esse arquivo para usar o padrão de 4 espaços dos arquivos XML

  • Movi a validação dos campo Código para somente o tamanho 2 do objeto abstrato para os objetos específicos, Código de Retorno e Instrução, para poder usa-lo em outros casos

  • Como foi visto no PR [14.0][RFC] l10n_br_account_payment_brcobranca - fields Santander CNAB 400 #2870 não é que existe uma Carteira para o Boleto e outra para Remessa, o campo pela descrição seria o "Tipo de Cobrança" ou "Código da Carteira" com tamanho 1

https://cms.santander.com.br/sites/WPS/documentos/arq-cobranca400-port/23-10-24_142650_h7800layoutdecobranca_4002023.pdf

image

image

Para resolver eu pensei em usar dois campos do tipo Seleção mas acabei vendo que no Bradesco 240 tem algo semelhante https://banco.bradesco/assets/pessoajuridica/pdf/4008-523-0687-layout-multipag.pdf

image

Por isso acabei optando por criar um novo objeto "Boleto Wallet Code" e assim caso exista outro caso semelhante vai ser possível usar essa mesma abordagem.

Pensando sobre isso acredito que a melhor maneira de atender o CNAB vai ser criando objetos para o Código de Desconto, Protesto e outros ao invés de apenas um CHAR porque assim o usuário vai pode identificar o campo pela nomenclatura usada pelo Banco, mas preciso testar e fazer isso em outro PR.

  • O Arquivo Remessa do Santander 400 precisa ser criado com o "Nosso Número + DV", foi feito um PR no BRcobranca buscando resolver isso e foi apontando a contradição em ter para esse mesmo banco no caso do 400 a necessidade de informar o DV mas não precisar no 240, mas o mantedor da biblioteca não aceito a alteração. A melhor solução talvez seja ver a possibilidade de incluir ou fazer algo semelhante ao git-aggregator https://github.com/acsone/git-aggregator na API e com isso incluir um commit de outro repositório que faça essa simples alteração porém mantendo a API ligada diretamente ao repo principal do BRcobranca, já que não existe o interesse em manter um Fork e um simples commit resolve o problema, a solução feita aqui é diferente do que foi feito na v12, aqui ao invés de salvar o campo no momento da geração do Boleto em um novo campo o calculo esta sendo feito apenas para esse caso e no momento de gerar o Arquivo Remessa( o método do calculo foi baseado em https://github.com/erpbrasil/erpbrasil.febraban/blob/master/src/erpbrasil/febraban/boleto/base.py#L44 ) , se necessário ou isso for útil para outros casos pode ser visto de migrar esse calculo do modulo11 para um lugar genérico e facilitar seu uso exemplo l10n_br_account_payment_order/tools.py

  • Renomeado os campos code_convetion para cnab_company_bank_code e code_convenio_lider para convention_code, essa alteração busca evitar duplicação de campos devido a falta de nomenclatura do CNAB, em todos os casos vistos existe um campo principal que tem um tamanho variado mas em muitos casos tem o tamanho 20, chamado G007 e em outros casos existe um segundo campo que também representa uma relação entre a empresa e o banco com tamanho 7, nesse caso do Santander devido o campo tamanho 20 ser chamando no código de Convenio acabou causando uma confusão e o campo acabou sendo usado para guardar esse segundo valor e acabou sendo criado outro campo para armazenar o campo tamanho 20 que é chamado de Código de Transmissão, para evitar ter vários campos eu procurei deixar um nome genérico e na visão nos casos conhecidos estou alterando para o nome usado pelo banco deixando a questão transparente para o usuário, nesse mesmo sentido acabei reutilizando o campo que antes era exclusivo para o Banco do Brasil o Código de Convênio Lider para usar no Santander e outros casos similares.

  • Resolvi a questão da necessidade de manter o objeto bank.payment.lines para evitar erro no banco de dados

  • Alterei no l10n_br_account_payment_brcobranca/models/account_payment_order.py a formação do metodo dos casos especificos para tirar o 240 e 400 e passar isso como paramentro para evitar codigo duplicado quando esses casos forem semelhantes em um mesmo banco

  • Habilitei o caso Santander 240 no BRCobranca, adicionado recentemente na lib, e foram incluídos dados de demonstração Modos de Pagto e Faturas para esse caso, será importante ver de incluir arquivos de Remessa e Retorno para testes

IMPORTANTE: Não atualize diretamente uma base de dados, veja de testar em um ambiente de homologação em caso de problemas veja de colocar o LOG aqui, eu procurei testar com dados de demonstração mas dependendo pode ser necessário alguma outra correção

cc @renatonlima @rvalyi @marcelsavegnago @mileo @antoniospneto @kaynnan

@OCA-git-bot
Copy link
Contributor

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

@mbcosta mbcosta force-pushed the 14.0-REF-adapting_cnab_santander_240_400 branch from 67b1008 to 3b1713a Compare February 24, 2024 00:56
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.

LGTM

@mbcosta
Copy link
Contributor Author

mbcosta commented Feb 27, 2024

valeu @marcelsavegnago @kaynnan @renatonlima , obrigado pela revisão, inclui mas três novos commits mas que não devem interferir no que vocês já haviam revisado, são referentes a:

  • Configuração do pre_commit para exclusão da pasta l10n_br_account_payment_brcobranca/tests/data porque se não acabava alterando os arquivos TXT da Remessa que foram adicionados para o caso Santander, isso existe na v12 https://github.com/OCA/l10n-brazil/blob/12.0/.pre-commit-config.yaml#L6 mas faltava na v14

  • Testes para os Códigos do CNAB no l10n_br_account_payment_order

  • Testes do caso Santander 400 e 240 geração do Boleto e Arquivo Remessa no l10n_br_account_payment_brcobranca

Por algum motivo os testes estão rodando a cerca de 2 Hrs sem terminar, não é possível ver no LOG se tem algum problema, eu busquei simular esse mesmo teste localmente e não tive erros, dependendo eu vou ver de fazer um rebase e fazer um Force Push para reiniciar os testes

https://github.com/OCA/l10n-brazil/actions/runs/8068908548/job/22042725608?pr=2925

image

Olhando o initlog https://runboat.odoo-community.org/api/v1/builds/bb1a8d490-3458-4dda-8b36-ed36bee72283/init-log parace ter um problema no modulo l10n_generic_coa

image

image

Mas ainda não sei se tem relação

@marcelsavegnago
Copy link
Member

marcelsavegnago commented Feb 28, 2024

@mbcosta não que esta informaçao ajude, mas eu já tive este erro do CheckViolation em algum situação.. se nao me engano era na preparação de ambiente quando instalava varios modulos da localização de uma unica vez. Na época acabei contornando preparando o ambiente em duas etapas igual o runboat faz.

@mbcosta
Copy link
Contributor Author

mbcosta commented Feb 28, 2024

valeu @marcelsavegnago , sim o último teste que faço é em duas etapas para simular o ambiente do runboat, mas aqui parece ter ocorrido outro problema e pelo LOG o erro foi no teste do IBPT l10n_br_fiscal/tests/test_ibpt.py https://github.com/OCA/l10n-brazil/actions/runs/8068908548/job/22061283488?pr=2925#step:8:348

image

Talvez o tempo que ficou rodando foram as tentativas de conexão, talvez seja melhor fazer um MOCK desse teste.

Os testes referentes a esse PR parece estar sem erros e pode ser vistos aqui https://github.com/OCA/l10n-brazil/actions/runs/8068908548/job/22061283488?pr=2925#step:8:2635

image

Vou ver de fazer um rebase para forçar os testes rodarem novamante

…tions and Return Move Codes for Santander Bank CNAB 400 and 240, changed the file for using four spaces as the standard for XML files.
…e codes from abstract model to specific ones to allow using this model for other cases.
…o 'Wallet to print in Boleto' in this case for both 240 and 400 there is a Wallet Code, and also for Bradesco 240, for now there this three cases mapped, by this reason the better form to solve seems to be the creation of a new object Boleto Wallet Code that allow attend this cases and, if exist, others.
…no 'Wallet to print in Boleto' in this case for both 240 and 400 there is a Wallet Code, and also for Bradesco 240, for now there this three cases mapped, by this reason the better form to solve seems to be the creation of a new object Boleto Wallet Code that allow attend this cases and, if exist, others.
…ranca need to send 'Nosso Numero + DV', the solution is a different approuch of what was made in v12 because it's seems specific case, Santander 240 don't need, by this reason the calculation are made only for this case during the creation of File.
…cnab_company_bank_code and code_convenio_lider to convention_code in order to avoid duplication of Fields because CNAB has no standards for the Name of the Fields.
…ntion to cnab_company_bank_code and code_convenio_lider to convention_code in order to avoid duplication of Fields because CNAB has no standards for the Name of the Fields, changed the method to generate CNAB file to avoid duplicate methods when the case 240 is similar with 400 and update the module to use the case of Santander 240.
@mbcosta mbcosta force-pushed the 14.0-REF-adapting_cnab_santander_240_400 branch from f03e8f7 to eb26d5e Compare February 28, 2024 20:55
@mbcosta
Copy link
Contributor Author

mbcosta commented Feb 28, 2024

Parece que o erro é referente ao teste que inclui do caso Santander https://github.com/OCA/l10n-brazil/actions/runs/8086952263/job/22097860177?pr=2925

image

O teste local usa a API do BRCobranca mas aqui ocorre apenas um MOCK preciso ver se faltou algo

@mbcosta mbcosta force-pushed the 14.0-REF-adapting_cnab_santander_240_400 branch from eb26d5e to 4e4ffc5 Compare February 28, 2024 22:27
@OCA-git-bot
Copy link
Contributor

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
Copy link
Member

rvalyi commented Mar 1, 2024

Ola @mbcosta parece que deu tudo certo com os testes agora. Ta tudo OK para vc agora neste PR?

@antoniospneto
Copy link
Contributor

antoniospneto commented Mar 4, 2024

@mbcosta @rvalyi

No futuro a gente poderia ver para fazer o CI rodar o a API do brcobrança localmente a a gente não precisar mockar o retorno, penso que por ser um serviço local seria bem mais seguro a gente ter as respostas reais, será que é muito complexo ?

@rvalyi
Copy link
Member

rvalyi commented Mar 4, 2024

poderia rodar. A unica questão é que nos obrigaria a alterar o CI bastante comparando com o padrão da OCA...

@rvalyi
Copy link
Member

rvalyi commented Mar 4, 2024

/ocabot merge major

@OCA-git-bot
Copy link
Contributor

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

@OCA-git-bot OCA-git-bot merged commit 3ad31ee into OCA:14.0 Mar 4, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

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

@mbcosta
Copy link
Contributor Author

mbcosta commented Mar 5, 2024

obrigado a todos pela revisão, @rvalyi estava tudo certo sim. o problema nos testes era o nome do arquivo dos Boletos que estavam errados e gerava erro, desculpe não ter respondido antes acabei focando em resolver um problema do l10n_br_stock_account

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.

7 participants