-
-
Notifications
You must be signed in to change notification settings - Fork 248
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][RFC] l10n_br_account_payment_brcobranca - fields Santander CNAB 400 #2870
[14.0][RFC] l10n_br_account_payment_brcobranca - fields Santander CNAB 400 #2870
Conversation
…leto_wallet_to_impress and change transmission_code to code_convetion - Santander CNAB 400
… attrs invisble for boleto_wallet_to_impress - Santander CNAB 400
@@ -21,7 +21,7 @@ class L10nBrCNABBoletoFields(models.AbstractModel): | |||
code_convetion = fields.Char( | |||
string="Código do Convênio no Banco", | |||
size=20, | |||
help="Campo G007 do CNAB", | |||
help="Codigo da Empresa/Beneficiario/Convenio/Transmissão", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
por mim tudo bem, mas eu confesso que achei um pouco estranho o tratar o "código de trasmissão" no mesmo campo do "código do convênio" pois como pode ver ali até o propria lib brcobrança trata de forma separada.
Apesar de que qualquer um desses campo de ser usado como identificador da empresa no banco, são informações diferentes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Então, acabei refatorando dessa maneira por conta dessa revisão Backport Santander, @mbcosta, como ficaríamos em relação a esse campo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@antoniospneto @kaynnan acredito que a lib BRCobranca procura criar os campos de acordo com o nome definido pelo Banco o que é bom lá para ficar de acordo com a Documentação do Banco e permitir um De Para direto, mas a lib não foi pensada para armazenar dados, os campos Código da Empresa/Beneficiario/Convenio/Transmissão são todos referentes a um:
- Campo que tem o código que identifica a Empresa no Banco e que tem o tamanho 20
Mas o nome do campo não é o mesmo entre os Bancos, devido a falta de padrão do CNAB mesmo na nomenclatura dos campos , por isso até onde vi o Antonio está errado em dizer que "são informações diferentes.", já que todos armazenam um campo com mesmo objetivo, ou esse campo código de transmissão não é o campo com o tamanho 20 que armazena um código que identifica a empresa junto ao banco?
Apenas para referencia sobre o debate na época #768
Isso também pode ser visto no código usado hoje https://github.com/OCA/l10n-brazil/blob/14.0/l10n_br_account_payment_brcobranca/models/account_payment_order.py , veja a lista de nome do campo de acordo com o Banco:
- convenio Banco do Brasil 400, Caixa Economica Federal 240 e AILOS 240
- codigo_beneficiario Banco UNICRED 400
- codigo_empresa Banco Bradesco 400
- codigo_transmissao Banco Santander 400
Deveríamos criar quatro campos diferentes para isso? Acredito que não, porque não faz sentido a Localização criar diversos campos para o CNAB que contém a mesma informação porém tem nomes diferentes entre os Bancos, como já foi falado não há um padrão nesses nomes, exemplo de outro campo em um caso é codigo_carteira em outro é carteira, deveríamos crias dois campos para esse caso? Esse De Para na localização acaba ficando no modulo l10n_br_account_payment_brcobranca
Entendo que pode ficar confuso para o desenvolvedor e por isso nos podemos pensar em alterar o nome do campo para algo genérico exemplo boleto_bank_company_code ou algum outro nome e colocar um comentário no código para evitar confusões, e também entendo que pode ficar confuso para o usuário e podemos ver de acrescentar um placeholder e talvez seja possível na visão incluir o campo 4 vezes com a String de acordo com cada caso e invisíveis entre si.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Entendi, por esse lado realmente não faz sentido, por conta que teria que especificar para qual campo é destinado a X Banco e o size dele é igual ao do code_convetion, sendo assim, não fica confuso para o Usuário e Desenvolvedor, então o ideal seria manter essa refatoração removendo o transmission_code e considerar o code_convetion para a transmissão/convenio etc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, mas lembrar que pode ter a necessidade de fazer script de migração entre as versões major 12>14.0 por causa do nome dos campos que mudaram.
escrevi ali minha observação sobre o campo transmission_code.
sobre o campo boleto_wallet2 realmente talvez não estava tão explicito no nome, mas no help tava sendo informado bem certinho o porque de existe um segundo campo para a carteira, penso que o usuario iria entender bem.
<field name="boleto_wallet2" /> | ||
<field | ||
name="boleto_wallet_to_impress" | ||
attrs="{'invisible': [('bank_code_bc', 'not in', ('033') )]}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
esconder o campo é bom que a visão fica mais limpa mas ao mesmo tempo tem que ter mais cuidado,
por exemplo tem que limpar o valor do campo caso ele esteja invisivel e ao mesmo tempo tenha valor, pra não ficar dado incosistente.
talvez só com a explicação do help o usuário técnico que tá configurando o banco já consegue saber se deve ou não preencher esse campo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nesse caso, acha viável complementar o help desse campo, incluindo que o mesmo é utilizado somente para o Santander após remover o atributo de invisibilidade? @mbcosta @antoniospneto
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@antoniospneto @kaynnan até onde entendi e pelo o que foi falado esse campo está sendo incluído para o caso especifico do Santander 400, ou não? Quais seriam os outros casos em que isso é usado? É preciso identifica-los, porque se isso é algo especifico para esse caso isso não deve afetar os outros casos ( se foi feito algum código nesse sentido vai precisar ser revisto ), a implementação foi pensada para permitir as diferenças entres os CNAB mas isolando o que é especifico de cada um, e se o desenvolvedor já sabe antecipadamente que um campo só existe para atender um determinado caso é desnecessário deixar para outra pessoa reavaliar isso em uma implementação, posso dizer que não é necessário para o caso UNICRED 400 então porque o usuário deveria ver esse campo? Isso vai além de deixar a visão "limpa" a implementação busca a medida que novos Bancos CNAB são implementados identificar as diferenças entre eles de forma clara para que os desenvolvedores possam tanto avaliar refatorações na Localização quanto no BRCobranca.
Outra questão acabei sugerindo o nome boleto_wallet_to_impress mas parece que essa tradução não é das melhores e talvez seja melhor algo como boleto_wallet_to_print, sugestões são bem vindas
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pelo o que compreendi através do backport que fiz da v12 referente a PR do @antoniospneto, foi que esse campo será destinado somente ao Santander 400, tanto que nessa refatoração alterei a declaração dele para o final do modelo l10n_br_cnab_boleto_fields.py
abaixo de outros campos para bancos específicos.
@kaynnan com o merge do PR #2871 ficou possível adicionar Dados de Demonstração e testes referentes a esse caso, isso não é obrigatório, e não sei se você usa ou tem esse caso homologado mas se tiver seria importante inclui-los, alterando os dados para torna-los anônimos, isso possibilita que os desenvolvedores façam testes e vejam os erros e diferenças desse caso, os testes também servem para garantir que um caso Homologado não sofra regressões e com isso também permite fazer refatorações sem preocupações de "quebrar" algo. |
@kaynnan @antoniospneto para buscar entender melhor, o caso Santander que vocês implementaram é o 240 ou 400? Acredito que seja o 400 porque o 240 foi incluído recentemente no BRCobranca mas é importante vocês confirmarem, se sim qual o valor está sendo informado nos campo "Carteira" e "Carteira 2"? Porque olhando na documentação e outras referencias na internet surgiram algumas questões:
Se for isso o ideal é saber se existe uma relação direta entre o valor da Carteira e esse campo "Tipo de Cobrança" porque aí pode ser feito um mapeamento direto sem necessidade de criar um campo mas se for preciso criar um campo parece que o melhor é criar um do tipo Selection/Seleção Outras referencias que encontrei na internet: A biblioteca erpbrasil.febraban também fazia um tipo de tratamento, é aqui parece que existe uma relação entre a Carteira e o Tipo de Cobrança Mesmo no BRCobranca no caso Itau 400 existe esse mapeamento para um código relacionado com a Carteira https://github.com/kivanio/brcobranca/blob/master/lib/brcobranca/remessa/cnab400/itau.rb#L67 Outra lib que aparece o problema Outra Lib com orientação de preenchimento de um campo parecido com o Seleção O caso 240 tem mais opções
400 Mas no 240 esse campo tem o tamanho 15 Talvez ainda seja possível usar o campo code_convetion fazendo uma validação para esse caso do tamanho 15, na própria implementação feita no BRCobranca isso está sendo feito https://github.com/kivanio/brcobranca/blob/master/lib/brcobranca/remessa/cnab240/santander.rb#L12 https://github.com/kivanio/brcobranca/blob/master/lib/brcobranca/remessa/cnab240/santander.rb#L87 https://github.com/kivanio/brcobranca/blob/master/lib/brcobranca/remessa/cnab240/santander.rb#L134 Bom aguardo a resposta de vocês para entender melhor esse caso |
@mbcosta na época que eu fiz a implementação foi homologado junto ao banco com essas configurações: Era cnab 400 e o codigo da carteira no arquivo de remessa era "05" e impresso no boleto era "101" Confesso que na época eu não me aprofundei muito, realmente é bem complexo entender a particularidade de cada banco. |
@antoniospneto buscando entender melhor essa diferença entre o Código de Transmissão e Convenio, No BRCobranca na impressão do Boleto está usando um campo com o nome convenio com tamanho 7 https://github.com/kivanio/brcobranca/blob/master/lib/brcobranca/boleto/santander.rb Na Remessa o nome usado é Transmissão com a questão do tamanho 20 https://github.com/kivanio/brcobranca/blob/master/lib/brcobranca/remessa/cnab400/santander.rb Na documentação do Código de Barras do Boleto está sendo chamado de Código do Beneficiário Pelo o que está escrito 1o. campo: composto pelo código do banco, código da moeda, campo fixo "9", quatro Parece que o Código que vai no Boleto ( no BRcobranca chamado de Convenio ) é composto por partes do Código de Transmissão ( Beneficiário pela documentação ) "quatro primeiras posições do código do beneficiário padrão Santander e dígito verificador deste campo." Não entendi esse digito verificador ( será que é o último número do Código de Transmissão? ) porque o tamanho do campo passado é 7 e se depois do 4 teria DV e depois mais 3 ficaria o tamanho 8. Bom apenas para tentar ver se tem alguma lógica nisso, você recebeu esses dois Códigos o Convenio e Transmissão de forma separada? É possível ver alguma semelhança entre esses códigos? Teria como verificar se no caso do Convenio realmente os 4 primeiros números são iguais ao de Transmissão? E o 5 número, esse DV, está também no de Transmissão é o último número? A sequencia dos 3 últimos números do Convenio também existe no de Transmissão? Dependendo podemos tentar usar essa lógica e evitar a necessidade de criar um novo campo, se não temos que ver qual campo deve ser usado para que, buscando evitar confusões por esses problemas de nomenclatura tanto para o usuários quanto para os desenvolvedores. |
@marcelsavegnago você ainda atende esse cliente do santander? antes de dar continuidade com a pr recomendo testar em produção se as alterações não vão gerar nenhum problema |
@antoniospneto sobre: "mas no manual o banco não deixa isso explicito, no caso mesmo que a gente encontre uma relação não quer dizer que o banco irá manter essa relação sempre." "Em nenhum lugar o banco menciona que essas informações podem ser calculadas ou extraida a partir de outras informações," No comentário anterior tem a referencia, mas para buscar esclarecer segue novamente Manual do Código de Barras do Santander do Boleto, página 5 É possível identificar no BRCobranca que depois do 9 vem esse código também https://github.com/kivanio/brcobranca/blob/master/lib/brcobranca/boleto/santander.rb#L82 Mas não vejo problema em ter esse campo para o "Convênio", era apenas para ver se existia a possibilidade de não precisar criar um novo campo. Eu estou buscando resolver esse caso na v14 e pretendo fazer as seguintes alterações para atender:
Já existia um pedido para isso devido erro de tradução, na época da implementação acabei adotando essa nomenclatura de "Convênio" para esses campos que tem o código que identifica a empresa no banco e tem o tamanho ou no máximo 20, mas acredito que para evitar a duplicação de campos é melhor ter um nome genérico e na visão incluir o mesmo campo mais de uma vez porém alterando o atributo String para cada caso onde seja "Código do Beneficiário" ou "Código do Convênio" ou "Código Transmissão" ou qualquer outro nome e assim isso fica "transparente" para o usuário, com isso o campo transmission_code pode ser excluído e essa informação passa a ser colocada nesse campo, vou buscar deixar comentado no código sobre essa questão.
Para atender os casos em que além do código que identifica a empresa no banco e tem o tamanho ou no máximo 20 existe também um segundo código que tem o tamanho 7, como nesse caso do Santander e do Banco do Brasil, mas com objetivo de atender qualquer outro caso semelhante e também alterando na visão o atributo String para adaptar para cada caso, e assim reutilizar o campo evitando a necessidade de criar novos campos em duplicidade. Sugestões são bem vindas, o objetivo é evitar a criação de diversos campos que são semelhantes mas que por uma questão de falta de padrão de nomenclatura do CNAB acabam tendo nomes diferentes. |
@kaynnan tem uma questão que não está diretamente ligada ao código que é o uso da sigla RFC, até onde sei isso é referente a Request for Comments em uma tradução simples seria uma Requisição para Comentários https://pt.wikipedia.org/wiki/Request_for_Comments pelo o que vejo isso normalmente é usado por aqui em Issues porque é uma fase anterior ao código, é quando ocorre o debate sobre algo que se pretende implementar e é feito um levantamento e se determina uma especificação, como deve ser ou qual a melhor maneira de fazer uma determinada implementação. E apenas para deixar claro o uso das outras siglas que são usadas por aqui:
|
Considerando que no Santander, "código de transmissão" e "código do beneficiário" precisam ser separados, assim como no BB, "código do convênio" e "código do convênio líder". qual seria a sua proposta? eu acho que renomear alguns pode ser valido, mas ainda vamos precisar ter campos destintos para os dois casos. |
Santander
A semelhança está no tamanho também ser 7, mas talvez seja melhor um nome genérico para esse campo para tirar a ligação direta com Convênio/convention mas não encontrei ainda algo nesse sentido e dependendo no futuro isso pode ser feito ( talvez renomear para cnab_company_bank_code_7 ? É preciso avaliar ) Banco do Brasil
Como no caso do Banco do Brasil essas informações já estão nesses campos a migração não vai afetar esse caso e a alteração parece atender ambos os casos. |
Compreendo sua preocupação, mas questiono se realmente vale a pena essa complexidade para generalizar os campos. Na minha visão, ter campos específicos parece ser mais flexível e menos confuso, permitindo tratar as particularidades sem muitos efeitos colaterais. Além disso, existe a possibilidade de um banco alterar o tamanho do campo ou alguma regra de validação futuramente. |
Entendo o que quer dizer, mas essa questão de generalizar esses campos é algo relativo e realmente não é possível ser feito para todos os caso, isso varia, estou buscando fazer onde parece ser possível e incluindo comentários no código sobre isso, mas se fossemos seguir a ideia de criar campos específicos para cada caso onde o nome do campo é diferente do existente teremos que criar diversos campos porque não existe um padrão na nomenclatura e em vários casos os campos são semelhantes tanto em tamanho quanto na informação que se referem, e ser for criado um campo para cada caso na minha opinião acabaria sendo muito mais confuso já que aumentaria a quantidade de campos e seria possível notar uma grande duplicidade entre esses campos, durante a implementação devido essa quantidade de campos foi criado um objeto abstrato para separar e armazenar apenas esses campos do CNAB https://github.com/OCA/l10n-brazil/blob/14.0/l10n_br_account_payment_order/models/l10n_br_cnab_boleto_fields.py e hoje já passa de 300 linhas. Um resumo é que é preciso considerar essa falta de padrão em qualquer implementação do CNAB( no caso de uma implementação de uma integração com uma LIB o ideal é fazer mais de um caso para visualizar essas divergências ) e é melhor não importar essa falta de padrão do CNAB para código na OCA porque isso não é algo bom na verdade é um grande problema do CNAB e o melhor é deixar isso separado fazendo o mapeamento o De Para em um lugar especifico como ocorre no l10n_br_account_payment_brcobranca onde ali na hora de gerar o Boleto ou o Arquivo de Remessa esses campos acabam sendo informados com a nomenclatura usada pelo Banco/CNAB. Como nesse caso do Santander por exemplo em todos os CNABs vistos até agora existe um campo que armazena o código de identificação da empresa junto ao banco e tem vários nomes Código da Empresa/Convênio/Beneficiário/Transmissão se for criado um campo para cada caso já seriam mais 3 campos novos, o Código do Convênio do Santander é semelhante em tamanho com o Código do Convênio Líder do Banco do Brasil então pode ser reutilizado para atender esse caso, por outro lado o campo do Tipo de Cobrança ou Código da Carteira é exclusivo do Santander não é possível reutilizar outro campo e pelo o que vi será preciso ter 2 campos do tipo Seleção um para o 400 e outro para o 240 porque o segundo tem mais opções, outro ponto pelo o que está no PR da v12 para o Arquivo de Remessa ser gerado corretamente é preciso enviar o Nosso Número + DV vi que foi feito um PR no BRCobranca buscando resolver e eu também tentei argumentar lá que mesmo no caso do Santander 240 isso não é necessário e que seria melhor a Lib tratar essa questão mas o Mantedor não aceitou, a melhor forma de resolver talvez seja ver uma forma de fazer na API algo semelhante com o que é feito com git-aggregator https://github.com/acsone/git-aggregator porque um simples commit de outro repo resolveria mas mantendo uma relação direta entre a API e o repo principal do BRCobranca, já que não existe o interesse em fazer ou manter um Fork do BRCobranca, mas para contornar o problema o que pretendo fazer é diferente do que foi feito na v12 que é isolar esse caso calculando o DV no momento de gerar o Arquivo e apenas para o caso Santander 400. Outro caso, por exemplo o campo boleto_protest_code https://github.com/OCA/l10n-brazil/blob/14.0/l10n_br_account_payment_order/models/l10n_br_cnab_boleto_fields.py#L95 apesar de na implementação ter sido visto que poderia ser um campo tipo Seleção acabei deixando Char para buscar ter compatibilidade com outros possíveis casos, mas é importante notar que a cada novo CNAB ou nova biblioteca ou API de Banco que é implementado ocorre uma re-avaliação da estrutura buscando a melhor compatibilidade possível e também ocorre uma maior compreensão do panorama geral e alguns pontos que eram dúvidas acabam sendo esclarecidos e por exemplo no futuro podemos chegar a conclusão que esse campo é um padrão e pode ser alterado para Seleção, o que não foi possível inicialmente devido a quantidade de casos e com isso a quantidade de horas que seriam gastas para incluir todos ou a maioria dos casos para ter esse panorama geral. Por outro lado olhando a documentação do Santander 240 acabei vendo que o campo "Boleto Especie" que está como Seleção e que eu considerei padrão na verdade também não é Campo atual https://github.com/OCA/l10n-brazil/blob/14.0/l10n_br_account_payment_order/constants.py#L482 É possível ver que isso é uma resquício dos módulos originais porque usavam um caso especifico como "O Padrão" que no caso era o Itau 240 No Santander 400, é possível ver diferenças mas o principal é que o 01 é referente a Duplicata Mercantil então tudo certo ainda é funcional No Santander 240, aqui não tem 01 a Duplicata Mercantil é 02 o que faz com que devido esse caso, por enquanto, a implementação do campo "Boleto Especie" vai precisar ser revista, ou criar um novo campo ou criar um objeto Bom acredito que até amanhã eu consigo subir um PR aí vai ser possível avaliar algo concreto. |
cc @marcelsavegnago @mbcosta @rvalyi @antoniospneto
Refatoração do código referente ao CNAB 400 do Santander, revisado pelo @mbcosta na PR #2848