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][IMP] l10n_br_account_payment_brcobranca: add Santander cod_desconto #3121

Conversation

kaynnan
Copy link
Contributor

@kaynnan kaynnan commented Jun 13, 2024

cc @marcelsavegnago @rvalyi @mbcosta

Essa PR propõe a inclusão do código de desconto na preparação das linhas do Santander.

Ao definir um percentual de desconto, observei que o arquivo de remessa no Segmento P posição 142 não está retornando o código de desconto conforme esperado, especificamente o código "1".

Após analisar o modelo em questão, percebi que existe uma condição que deveria atribuir o valor "1" ao código de desconto no arquivo de remessa quando um valor de desconto é especificado:

if self.discount_value:
    # Código adotado pela FEBRABAN para identificação do desconto.
    # Domínio:
    # 0 = Isento
    # 1 = Valor Fixo
    linhas_pagamentos["cod_desconto"] = "1"

Retorno do Santander referente a ausência do código de desconto:

image

@OCA-git-bot
Copy link
Contributor

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

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

@mileo
Copy link
Member

mileo commented Jun 13, 2024

Essa alteração já foi feita no brcobrança? Pois não vi explicitamente o uso do cod_desconto no santander.

Se isso for para cnab240 então não deve ficar nesse método e sim no geral para todos os bancos com 240 e também para a unicred 400

image

@kaynnan
Copy link
Contributor Author

kaynnan commented Jun 13, 2024

Essa alteração já foi feita no brcobrança? Pois não vi explicitamente o uso do cod_desconto no santander.

Se isso for para cnab240 então não deve ficar nesse método e sim no geral para todos os bancos com 240 e também para a unicred 400

image

Olhei aqui, a função do código de desconto existe para o Remessa 240 do Santander.

image

Mas pelo l10n_br_account_payment_brcobranca sempre retornava 0 mesmo com o percentual modificado, vou alterar o código para ter o comportamento somente no CNAB 240 do Santander, já que pelo o que olhei aqui na estrutura da Remessa do 400 não tem essa função definida por lá

@kaynnan kaynnan force-pushed the 14.0-imp-l10n_br_account_payment_brcobranca-santanderdesconto branch from dc7a6ce to e460a9e Compare June 13, 2024 15:09
@kaynnan
Copy link
Contributor Author

kaynnan commented Jun 13, 2024

Fico no aguardo para definir essa questão, pois, ao analisar os arquivos de remessa de cada banco (kivanio/brcobranca), constatei que alguns não possuem a função de código de desconto definida. No entanto, na base da Remessa 240, essa função está presente e no Remessa CNAB 240 do Santander também.

image
image

@mileo
Copy link
Member

mileo commented Jun 13, 2024

@kaynnan como todos os bancos de 240 herdam de base.rb quer dizer que todos usam esse campo, o santander pode apenas implementar uma forma de tratamento diferente.

Em linhas gerais o boleto 240 é padronizado pela febraban e todos os banco seguem o mesmo layout, até pq a troca de informações entre os bancos tb é feita por esse arquivo (ou era no passado).

O layout 400 surgiu para melhorar a questão da complexidade de geração do arquivo, pois 240 colunas cabem poucas informações, mas não é um layout regulamentado pela FEBRABAN. Então os bancos acabam alterando de forma deliberada esse padrão para diversificar seus produtos.

Mas pode haver coincidências entre o 240, o 400 e entre os bancos. Portanto a manutenção desse código sempre deve levar em conta essas semelhanças e quando for alterar um banco, lembrar que algo pode ocorrer nos outros bancos de forma semelhante. No final das contas não tem um algoritmo definitivo para todos os bancos, mas com o tempo chegamos lá ou vamos todos para o PIX com vencimento.

@kaynnan
Copy link
Contributor Author

kaynnan commented Jun 13, 2024

@kaynnan como todos os bancos de 240 herdam de base.rb quer dizer que todos usam esse campo, o santander pode apenas implementar uma forma de tratamento diferente.

Em linhas gerais o boleto 240 é padronizado pela febraban e todos os banco seguem o mesmo layout, até pq a troca de informações entre os bancos tb é feita por esse arquivo (ou era no passado).

O layout 400 surgiu para melhorar a questão da complexidade de geração do arquivo, pois 240 colunas cabem poucas informações, mas não é um layout regulamentado pela FEBRABAN. Então os bancos acabam alterando de forma deliberada esse padrão para diversificar seus produtos.

Mas pode haver coincidências entre o 240, o 400 e entre os bancos. Portanto a manutenção desse código sempre deve levar em conta essas semelhanças e quando for alterar um banco, lembrar que algo pode ocorrer nos outros bancos de forma semelhante. No final das contas não tem um algoritmo definitivo para todos os bancos, mas com o tempo chegamos lá ou vamos todos para o PIX com vencimento.

Entendi, nesse caso teria que refatorar o modelo para incluir por padrão esse cod_desconto?

Essa validação da preparação do santander fiz com base na preparação de outros bancos que declara essa validação no modelo do l10n_br_account_payment_brcobranca, exemplo o Ailos

@kaynnan
Copy link
Contributor Author

kaynnan commented Jun 17, 2024

Pessoal,

Como procederemos em relação a essa questão sobre o código de desconto para o CNAB 240 e 400?

@antoniospneto
Copy link
Sponsor Contributor

Pois é, na época eu apliquei isso somente no Ailos #2315, pois era onde eu tinha certeza da necessidade do campo. A fim de evitar regressão, por não conhecer a particularidade dos outros bancos, não implementei de forma geral.

Se for ver, o @mbcosta já tinha feito o mesmo para o Unicred.

Eu, particularmente, não acho tão feio se o código ficar duplicado nesses casos. No fim, cada banco tem sua implementação e cada um pode evoluir independentemente, sem o risco de a alteração de algo em um banco impactar indesejavelmente outro banco.
Até porque é extremamente difícil testar todas as possibilidades da integração CNAB com o banco, então as regressões podem ser percebidas só em produção. Manter esse isolamento pode ser bom.

@mileo
Copy link
Member

mileo commented Jun 18, 2024

Nesse caso, como é 240, o correto é vc deixar geral. No 400 lidamos banco a banco.

@kaynnan kaynnan force-pushed the 14.0-imp-l10n_br_account_payment_brcobranca-santanderdesconto branch 2 times, most recently from e3bc57c to 7447b0a Compare June 18, 2024 17:20
@kaynnan
Copy link
Contributor Author

kaynnan commented Jun 18, 2024

Realizei a mudança aqui, fiz essa validação dentro de:

# Cada Banco pode possuir seus Codigos de Instrução
        if (
            self.mov_instruction_code_id.code
            == payment_mode_id.cnab_sending_code_id.code
        ):

No final do código com:

if payment_mode_id.payment_method_code == "240":
              if self.discount_value:
                  # Código adotado pela FEBRABAN para identificação do desconto.
                  # Domínio:
                  # 0 = Isento
                  # 1 = Valor Fixo
                  linhas_pagamentos["cod_desconto"] = "1"
    ```

@antoniospneto
Copy link
Sponsor Contributor

antoniospneto commented Jun 18, 2024

Realizei a mudança aqui, fiz essa validação dentro de:

# Cada Banco pode possuir seus Codigos de Instrução
        if (
            self.mov_instruction_code_id.code
            == payment_mode_id.cnab_sending_code_id.code
        ):

No final do código com:

if payment_mode_id.payment_method_code == "240":
              if self.discount_value:
                  # Código adotado pela FEBRABAN para identificação do desconto.
                  # Domínio:
                  # 0 = Isento
                  # 1 = Valor Fixo
                  linhas_pagamentos["cod_desconto"] = "1"
    ```

Como tá sendo implementado de forma generica para todos os bancos, pode remover a parte do código que implementa isso diretamente no Ailos e Unicred

Edit:
Mas assim o que eu comentei antes, é que mesmo sendo 240 o que o FEBRABAN define é apenas uma recomendação ou boa pratica rsss.. pq na realidade é cada banco por si.. mas no geral muita coisa permanece o mesmo sim, só não pode considerar que vai ser sempre igual.. ainda tem a possibilidade de algum banco, mesmo sendo 240 tratar isso de forma diferente, em algum banco o valor de 1 pode corresponder a outra coisa e não ao "Valor Fixo" mas de qualquer forma não é algo tão perigoso, por mim é tranquilo seguir assim, se acontecer de dar problema em algum banco é facil resolver.

@kaynnan kaynnan force-pushed the 14.0-imp-l10n_br_account_payment_brcobranca-santanderdesconto branch from 7447b0a to c83ee52 Compare June 18, 2024 17:45
Copy link
Sponsor Contributor

@antoniospneto antoniospneto left a comment

Choose a reason for hiding this comment

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

LGTM!

@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 Jun 19, 2024

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

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

@OCA-git-bot OCA-git-bot merged commit fa81b27 into OCA:14.0 Jun 19, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

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

@kaynnan kaynnan deleted the 14.0-imp-l10n_br_account_payment_brcobranca-santanderdesconto branch June 19, 2024 01:18
@mbcosta
Copy link
Contributor

mbcosta commented Jun 19, 2024

valeu @kaynnan obrigado pelo PR, a minha ideia sobre esse e outros campos é cria-los como objetos para permitir tanto que cada caso tenha o seu conjunto de códigos e nomenclatura de acordo com o Banco/CNAB o que vai facilitar a identificação pelo usuário por mostrar tanto o Código quando a Descrição e vai ser possível agrupar os casos iguais, algo semelhante com o que foi feito no "Código da Carteira" #2925 inicialmente foi deixado esses e outros campos como CHAR para permitir o cadastro porém vendo o problema do código da carteira acredito a abordagem por objeto será melhor

@mileo alguns pontos importantes:

"Em linhas gerais o boleto 240 é padronizado pela febraban e todos os banco seguem o mesmo layout, "
"Mas pode haver coincidências entre o 240, o 400 e entre os bancos. "

Como você sabe e acompanhou houveram outras tentativas de implementação do CNAB houve um RFC e especificação do que precisava ser feito e o principal problema era que se partia do principio que haveria esse "padrão" e para isso era usado como referencia o CNAB 240 do Itau porém ao tentar implementar outros Bancos/CNAB era preciso fazer várias refatorações no código, existiam campos que eram colocados como hardcode baseados nessa ideia, então a cada novo banco se corria o risco de "quebrar" a implementação anteriores e os PRs acabavam alterando diversas partes do código, o que dificultava as revisões e aumentava a possibilidade de erros e isso só foi resolvido quando foi adotado como principio que não existe esse "padrão" que os Bancos usam códigos e nomenclaturas diferentes mesmo no 240 e alguns campos só existem em alguns casos, foram gastos meses nisso até chegar a essa conclusão e com essa mudança a implementação passou a ser mais robusta e menos suscetível a erros, você pode confirmar isso mesmo no 240 nos códigos do caso Santander 240 https://github.com/OCA/l10n-brazil/blob/14.0/l10n_br_account_payment_order/data/cnab_codes/banco_santander_cnab_240_400.xml e Itau 240 https://github.com/OCA/l10n-brazil/blob/14.0/l10n_br_account_payment_order/data/cnab_codes/banco_itau_cnab_240_400.xml algo simples no Santander a Instrução Código 10 é Concessão de Desconto e no Itaú é NÃO PROTESTAR (INIBE O PROTESTO AUTOMÁTICO) quer dizer são coisas completamente diferentes, é isso estava como hardcode antes.

A implementação busca evitar códigos duplicados e identificar o que é ou pode ser comum porém separando o que é especifico de cada Banco, se não sabemos se algo deve ou não ter um determinado valor é melhor considerar que não sabemos e não preencher do que voltar para essa ideia que "é o padrão" e que o campo deve ser preenchido em todos os casos.

Mesmo no caso desse PR pela imagem que foi colocada pelo o que entendi não existem apenas duas opções nesse caso são 5.

image

"Portanto a manutenção desse código sempre deve levar em conta essas semelhanças e quando for alterar um banco, lembrar que algo pode ocorrer nos outros bancos de forma semelhante."

Discordo completamente, se não se sabe se isso "pode ocorrer" ou não, coloque apenas no Banco que você está homologando, "pode ocorrer" deve ser desconsiderado achismo deve ser evitado, a implementação foi pensada para separar o que se sabe de cada banco para que uma nova implementação não interfira em outras e como você colocou a cada novo caso nos aumentamos o "Banco de Conhecimento" e se temos a informação de casos comuns não vejo problema em junta-los mas sem ter essa informação isso deve ser evitado, porque pode causar inúmeros problemas eu procurei deixar isso claro no REAME do modulo que o CNAB envolve o Financeiro da empresa e qualquer erro ou problema nisso não é algo que pode ser visto amanhã ou daqui uma duas semanas o desenvolvedor responsável vai precisar fazer uma manutenção de emergência e o fluxo da empresa pode precisar ser parado por algo que foi Homologado entre a empresa e o Banco o que é péssimo tanto para o desenvolvedor quanto para a empresa.

@antoniospneto o que você fez era o certo pelos motivos que escrevi acima mas não vejo problemas em juntar casos mapeados mas não concordo em juntar casos que não se sabe e apenas para reforçar "mas de qualquer forma não é algo tão perigoso, por mim é tranquilo seguir assim, se acontecer de dar problema em algum banco é facil resolver."

Independente de ser algo fácil ou não se a criação do CNAB parar ou passar a dar erro o fluxo financeiro de uma empresa pode parar e isso pode ter um custo baixo ou alto dependendo da empresa, uma empresa que emite 10 Notas por semana não é a mesma coisa de uma que emite 100 por dia, ou outro ponto uma coisa é uma empresa com erro porém aqui a maioria dos desenvolvedores tem mais de um cliente então se você tiver 30 clientes com problema no Fluxo Financeiro da empresas mesmo sendo rápido você vai precisar de horas para resolver todos os casos, será que esses clientes vão considerar que ficar sem poder Gerar o CNAB que eles homologaram e realizaram testes é algo razoável? Quanto estresse isso pode gerar entre a empresa/cliente e o desenvolvedor? Já que vai ser preciso uma manutenção de emergência, ou qual a imagem esse desenvolvedor/empresa passa para o seu cliente? Existe algum desenvolvedor que gosta de perder a madrugada ou fim de semana ou feriado resolvendo BUGs porque alguém "achou"? Nessa implementação foram incluídos diversos dados de demonstração e testes para os casos conhecidos e sempre recomendo que quem fizer uma implementação inclua os Codigos de Instrução de Movimento, Retorno, Dados de Demonstração e Testes para evitar qualquer problema ou regressões devido essas divergências entre os CNABs e assim buscar evitar qualquer possibilidade de manutenções de emergência já que é muito melhor trabalhar apenas com manutenções preventivas tanto para os desenvolvedores quanto para os clientes.

@rvalyi
Copy link
Member

rvalyi commented Jun 19, 2024

valeu pelo retorno @mbcosta eu fiz o merge sem esperar vc comentar porque o PR tava no ar ja por varios dias e na pior era algo facil de mudar caso vc achasse errado (nao tem mudança na estrutura de dados por examplo).

@antoniospneto
Copy link
Sponsor Contributor

antoniospneto commented Jun 19, 2024

@mbcosta sou da mesma opnião que você, mas enfim.. eu aprovei pra não ficar bloqueando o trabalho do @kaynnan por divisão de opnião. Empresa pequena ou grande nenhuma aceita parar as operações eu sei muito bem como é isso... por mais que a gente cuide, atualizar a localização é sempre um processo delicado.. mas que bom que você concorda comigo, valeu!

@mbcosta
Copy link
Contributor

mbcosta commented Jun 19, 2024

valeu @rvalyi @antoniospneto
Desculpem pelo atraso na revisão, o PR não deve causar erros mas acredito que é importante deixar claro os problemas na arquitetura do modulo e porque foi feito de determinada forma ao invés de outras, tanto para o uso do BRCobranca quanto para quem for implementar outras bibliotecas, talvez esteja faltando uma explicação melhor no README do l10n_br_account_payment_order sobre isso( um TODO para atualizar ), é normal que como desenvolvedores nos buscamos uma logica para reduzir o problema, mas uma arquitetura errada pode acabar levando a perda de horas/dias/meses até chegar a conclusão que a abordagem/arquitetura desse problema estava errada, por isso no caso CNAB existem dados de demonstração e testes com mais de um caso para que esse problema apareça e o desenvolvedor esteja atento a isso, talvez não seja necessário incluir todos mas se um determinado caso tem muitas diferenças o melhor é incluir nos testes para evitar regressões e manter a cobertura dos testes/coverage.

A melhor solução como escrevi pelo o que vi é transformar esse e outros campos que hoje são um genérico CHAR em um Objeto porque o usuário vai passar a ver o "Código - Descrição" de acordo com o que está no CNAB evitando dúvidas e permitindo também juntar os casos semelhantes, nesse caso talvez vai ser possível atender as outras possibilidades de Descontos o 2,3 e 4, mas isso não impede esse PR e pode ser feito em outro ( vou ver se consigo implementar, se alguém pretende fazer isso por favor avise para evitar trabalho duplicado ).

Também é importante dizer que apesar da falta de padrão nós podemos por exemplo buscar padronizar no BRcobranca o arquivo de retorno do CNAB nome de campos e Data para evitar tratamentos distintos o que pode reduzir código aqui na localização.

Apenas um exemplo do tamanho do problema não sei se a Documentação do Banco Nordeste CNAB 400 https://github.com/kivanio/brcobranca/tree/master/docs/BNB está desatualizada porém se não estiver o Código de Retorno é 51

image

Isso hoje está "chumbado/hardcode" aqui https://github.com/OCA/l10n-brazil/blob/14.0/l10n_br_account_payment_brcobranca/parser/cnab_file_parser.py#L374

image

Quer dizer que hoje para atender esse caso vai ser preciso verificar essa questão.

Uma outra questão mas ligada ao BRCobranca e aproveitando que aqui tem outros desenvolvedores usando isso, eu busquei reduzir o tamanho da imagem docker da API alterando do Debian para o Alpine, são menos 252 MB agradeço quem puder testar e fazer revisões akretion/boleto_cnab_api#33

@mbcosta
Copy link
Contributor

mbcosta commented Sep 12, 2024

Pessoal esse PR acabou incluindo uma regressão o caso UNICRED 400 passou a não enviar o Código de Desconto mesmo tendo valor, estou buscando resolver no PR #3360 , infelizmente hoje não existe uma forma de testar, porque o valor é preenchido apenas no momento da criação do Arquivo Remessa, estou considerando incluir esses e outros campos de Códigos no account.payment.line para poder validar isso e assim evitar regressões como essa.

@rvalyi
Copy link
Member

rvalyi commented Sep 12, 2024

Pessoal esse PR acabou incluindo uma regressão o caso UNICRED 400 passou a não enviar o Código de Desconto mesmo tendo valor, estou buscando resolver no PR #3360 , infelizmente hoje não existe uma forma de testar, porque o valor é preenchido apenas no momento da criação do Arquivo Remessa, estou considerando incluir esses e outros campos de Códigos no account.payment.line para poder validar isso e assim evitar regressões como essa.

sera se isso melhora #3361 ?

@mbcosta
Copy link
Contributor

mbcosta commented Sep 18, 2024

@rvalyi esse PR vai ajudar muito, mas não resolve esse problema porque a informação é apenas usada para criar o TXT e não é feita nenhuma validação nesse arquivo gerado, a única forma que vejo de poder testar isso é a que escrevi acima "incluir esses e outros campos de Códigos no account.payment.line para poder validar isso e assim evitar regressões como essa." isso também pode permitir que, se necessário, o usuário possa alterar os valores padrão para um determinado caso os Códigos de Desconto, Multa, etc, bom o PR de correção e que inclui os outros possíveis Códigos de Desconto #3360 já tem apenas os commits referentes o que deve facilitar a Revisão

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