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][ADD] l10n_br_cnab_structure: novo módulo para integração CNAB. (sem payment way) #2181

Merged

Conversation

antoniospneto
Copy link
Contributor

@antoniospneto antoniospneto commented Oct 21, 2022

Adiciona o novo módulo CNAB Structure.
O mesmo que proposto na #2168
Porém sem a funcionalidade do payment way, essa funcionalidade deve ser discutida melhorada e adicionada no futuro.

@antoniospneto antoniospneto changed the title [14.0][ADD] l10n_br_cnab_structure: novo módulo para integração CNAB. [14.0][ADD] l10n_br_cnab_structure: novo módulo para integração CNAB. (sem payment way) Oct 21, 2022
@antoniospneto antoniospneto marked this pull request as draft October 21, 2022 11:52
@antoniospneto antoniospneto force-pushed the 14.0-add-cnab-structure-without-payment-way branch from 2ccc27c to 3927901 Compare October 21, 2022 12:18
@marcelsavegnago
Copy link
Member

@netosjb tbm sugiriro dar uma limpada em alguns commits fazendo squash de alguns

@marcelsavegnago
Copy link
Member

No caso a ideia do squash seria para nao ter commit incluindo e commit removendo payment way senao pode gerar confusão no histórico do módulo

@antoniospneto
Copy link
Contributor Author

@marcelsavegnago vai ser ruim, o payment way foi introduzido aos poucos na branch, se eu fizer squash vou ter que unir quase tudo, vamos perder todo o histórico do branch.

@marcelsavegnago
Copy link
Member

@marcelsavegnago vai ser ruim, o payment way foi introduzido aos poucos na branch, se eu fizer squash vou ter que unir quase tudo, vamos perder todo o histórico do branch.

@rvalyi alguma sugestão ?

@rvalyi
Copy link
Member

rvalyi commented Oct 28, 2022

@marcelsavegnago vai ser ruim, o payment way foi introduzido aos poucos na branch, se eu fizer squash vou ter que unir quase tudo, vamos perder todo o histórico do branch.

@rvalyi alguma sugestão ?

Infelizmente não vejo nada simples a ser feito. Mas olha, todos módulos um pouco complexo como fiscal, NFe, account e CNAB tiveram tb uns passos fora nós commits. Nesse nível de complexidade infelizmente acontece, não acho problema neste caso.

@antoniospneto antoniospneto force-pushed the 14.0-add-cnab-structure-without-payment-way branch from fea7f12 to 8dd86f0 Compare October 28, 2022 16:59
@marcelsavegnago
Copy link
Member

Grande @netosjb e @felipemotter . PR #2180 mesclada. Podem fazer um rebase desta PR por favor ?

@felipemotter felipemotter force-pushed the 14.0-add-cnab-structure-without-payment-way branch from 6c36e13 to b3e1d78 Compare October 31, 2022 18:42
@felipemotter
Copy link
Contributor

@marcelsavegnago feito

@antoniospneto
Copy link
Contributor Author

Pessoal vou trabalhar na branch até ficar tudo verde, assim que tiver tudo ok volto aqui para marcar como pronta para revisão.

@@ -0,0 +1,130 @@
"id","name","meaning","start_pos","end_pos","assumed_comma","type","default_value","cnab_line_id/id","notes","sending_dynamic_content","content_source_field","return_dynamic_content","content_dest_field","cnab_group_id/id"
"l10n_br_cnab_structure.cnab_itau_240_header_1_3","CÓDIGO DO BANCO","CÓDIGO DO BCO NA COMPENSAÇÃO","1","3","0","Numeric","341","l10n_br_cnab_structure.cnab_itau_240_header","","","company_partner_bank_id.bank_id.code_bc","","",""
Copy link
Member

Choose a reason for hiding this comment

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

@netosjb Sugiro mudar o valor do campo type para a chave do selection e não a string. Estava testando a instalação na instancia DEV do cliente e só passou depois desta mudança.

image

Copy link
Member

Choose a reason for hiding this comment

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

Inclusive se tiver algum outro selection sendo populado pela string eu sugiro mudar para a chave.

@antoniospneto antoniospneto force-pushed the 14.0-add-cnab-structure-without-payment-way branch 3 times, most recently from d61edc9 to f1a0243 Compare November 5, 2022 19:29
@mbcosta
Copy link
Contributor

mbcosta commented Nov 7, 2022

Mesma revisão feita em #2168

@felipemotter @antoniospneto parabéns pelo trabalho, obrigado pelo vídeo explicando o PR, interessante a forma que vocês abordaram o problema e bem interessante o visualizador de TXT, mas é preciso dizer que a forma de implementação do CNAB na OCA vem sendo discutida, pelo o que vi e lembro, desde de 2015 e em 2017 houve um encontro da comunidade onde isso foi debatido, alguns queriam colocar nos módulos da localização o código que faria o mapeamento de cada CNAB ( https://github.com/odoo-brazil/odoo-brazil-banking/tree/8.0/l10n_br_account_banking_payment_cnab/febraban https://github.com/odoo-brazil/odoo-brazil-banking/blob/8.0/l10n_br_cnab_import/file_cnab240_parser.py ) e outros queriam usar bibliotecas externas, pelo o que lembro a primeira opção foi rejeitada porque seria melhor ter a implementação do CNAB "encapsulada" em uma biblioteca especifica, algo parecido com o CEP, NFe, etc; isso resulta em menos código na localização por ser necessário apenas um modulo que na medida do possível é simples e pequeno para integrar essa lib X; alterações, manutenções e atualizações ficam restritas a partes especificas e separadas do código, o que ajuda na gestão do repositório e isso atende o conceito de modularizarão, uma atualização de uma biblioteca especifica não afeta diretamente o repositório da localização e uma lib em python ou ruby ou outra linguagem em teoria pode ter mais contribuidores do que algo especifico do Odoo, também ficou definido que Localização deve permitir a possibilidade de uso de mais de uma biblioteca para o CNAB, a partir disso acredito que além da funcionalidade em si a arquitetura passou a ser também parte da especificação.

No vídeo vocês comentam sobre a possibilidade de juntar Bank Lines, isso foi evitado no CNAB pelo menos o de Cobrança até agora porque essas linhas e a payment.lines representam as linhas do CNAB onde cada uma tem um numero sequencial, acredito que o melhor é deixar isso como espelho do arquivo gerado mantendo a separação se esse for o caso do arquivo, até por motivo de rastreabilidade( lembro que havia uma recomendação mesmo no account_payment_order para não juntar as linhas quando o campo communication_type fosse do tipo "Estruturado", pelo o que entendo um comunicação que segue uma sequencia ), mas é importante ouvir vocês sobre isso.

Vi que na importação do arquivo existe um fluxo de Aprovação, eu gostaria de entender o que os levou a não importar de uma vez, se tem algo que precisa ser feito ou verificado nesse momento? O wizard que existia antes também era dessa forma porém no BRCobranca acabei decidindo alterar e importar de uma vez o aquivo sem fluxo, por uma questão de usabilidade diminuindo a quantidade de cliques para realizar uma tarefa mas principalmente por uma questão levantada de possibilidade de Fraudes, o que nos levou a pensar no que pode ser feito na implementação para evitar ou se possível não permitir ou ao menos registar tudo que possa ser usado para realização de fraudes, como vocês viram o CNAB é algo antigo que usa simples arquivos TXT e alguém com acesso pode altera-los, existem formas de proteger o arquivo fora do Odoo com pastas do tipo Somente Leitura/readonly, mas como o repositório é publico e terceiros podem estar usando é importante alertar sobre o problema tanto para a OCA , desenvolvedores do projeto e a comunidade se prevenirem de possíveis acusações de BUGs ou Falhas que podem vir a facilitar possíveis fraudes quanto esses terceiros/desenvolvedores ou mesmo "usuário funcional com um bom conhecimento técnico e pouca experiência python" entenderem a responsabilidade envolvida já que em casos onde é descoberta uma fraude existe uma cadeia de responsabilidades legais onde cada um responde por ação ou omissão, entendo a ideia de facilitar a implementação via tela do odoo mas é preciso deixar claro que como qualquer trabalho existe um grau de responsabilidade envolvido, principalmente no caso de comunicação com Banco, acredito que isso precisa estar claro para todos os desenvolvedores até para eles se protegerem de acusações de cooperação ou conivência nesses casos, por isso foi incluído em determinados campos de configuração do CNAB o parâmetro Tracking para assim registrar quem é quando fez uma determinada alteração para em caso de uma auditoria ou investigação seja possível rastrear as responsabilidades de cada um, também foi colocado um aviso no README sobre um problema que esse parâmetro tem em não registrar alterações nos campos do tipo Many2many e a recomendação para usar um modulo que corrigi essa falha já que é justamente o campo que define os Códigos de Retorno que devem gerar Baixas/Liquidações https://github.com/OCA/l10n-brazil/blob/14.0/l10n_br_account_payment_order/readme/CONFIGURE.rst. Sobre isso é importante destacar que na forma como vocês estão implementando se for preciso atender essa questão de monitorar campos importantes do CNAB vocês terão que incluir ou mail.tread e o parâmetro Tracking ou devido a quantidade de campos incluir a dependência do modulo auditlog https://github.com/OCA/server-tools/tree/14.0/auditlog e incluir arquivos data configurando esse modulo, e aqui existe uma vantagem na implementação que "encapsula" uma biblioteca porque dessa forma uma alteração no padrão dos arquivos CNAB é muito mais difícil de ser feita, alguém mal intencionado teria que ter o acesso ao servidor que roda a aplicação o que não é comum e entre Usabilidade X Segurança/Consistência de Dados eu acredito que o melhor é optar por Segurança/Consistencia de Dados.

Nesse encontro da comunidade em 2017 eu não conhecia bem o CNAB e nem as possíveis formas de implementa-lo na OCA e por isso apenas acompanhei o debate e não pude ajudar, a partir de 2018, orientado pelo @rvalyi e o @renatonlima , eu passei a ver e trabalhei no sentido de atender as especificações acima para deixar os módulos que existiam no repositório odoo-brazil-banking genéricos sem uma ligação direta com uma Lib ou código especifico de um CNAB dentro dos módulos( https://github.com/odoo-brazil/odoo-brazil-banking/pulls?q=is%3Apr+author%3Ambcosta+is%3Aclosed odoo-brazil/odoo-brazil-banking#68 ) e foi feito algo funcional mas a implementação do BRCobranca estava fora da OCA (https://github.com/akretion/odoo-boleto-cnab/tree/8.0 ), somente a partir de Junho de 2020 eu passei a ver com mais profundidade visando atender todas as especificações e assim incluir o modulo na OCA, gastei mais tempo do que o esperado e muitas vezes que achei estar próximo de terminar aparecia alguma nova variável que alterava a estrutura e era preciso repensar partes da implementação, o problema do CNAB é a falta de padrão entre eles, por isso para evitar que outras pessoas gastem horas/dias/meses eu me coloco a disposição de vocês ou outras pessoas que queiram implementar o CNAB na OCA para conversar via RFC ou mesmo marcar uma vídeo conferencia( agendando antecipadamente ) para explicar o que foi feito e como abordar essa implementação do CNAB dentro da OCA.

Minha conclusão é que como no passado a ideia de incluir a estrutura do CNAB diretamente nos módulos na Localização OCA foi recusada e a diferença que vejo é que vocês estão colocando isso na tela como cadastro e com arquivos Data ( https://github.com/Engenere/l10n-brazil/blob/14.0-cnab-structure/l10n_br_cnab_structure/data/l10n_br_cnab.line.field.csv ) ao invés de colocar no código como havia sido feito no odoo-brazil-banking ( https://github.com/odoo-brazil/odoo-brazil-banking/blob/8.0/l10n_br_account_banking_payment_cnab/febraban/cnab_240/cnab_240.py https://github.com/odoo-brazil/odoo-brazil-banking/blob/8.0/l10n_br_account_banking_payment_cnab/febraban/cnab_240/bancos/itau.py )e vendo que o Processador do CNAB foi chamado "OCA Processor" acredito que revisão será a mesma que foi feita no passado e o PR vai acabar sendo recusado e recomendado a integração com uma biblioteca existente ou nova, o que é o que hoje eu também penso devido os pontos já falados, mas claro vocês podem argumentar e buscar convencer os PSCs e outros desenvolvedores sobre o porque isso deveria ser revisto e o PR aprovado. Gostaria de pedir desculpas a vocês porque acredito que o README do l10n_br_account_payment_order deveria explicar melhor essa questão e pretendo incluir algo para tornar isso mais claro, todo esse debate acabou ficando restrito a um pequeno grupo de pessoas e penso que uma conversa em um RFC ou uma conversa informal de uma ou duas horas poderiam ter esclarecido diversas dúvidas de vocês e poupado horas/dias de desenvolvimento, por isso como escrevi acima estou a disposição para falarmos via vídeo conferencia, o CNAB de Pagamento é algo pendente que acredito que todos gostariam que seja incluído na Localização. Se efetivamente a proposta for recusada as recomendações que posso fazer são as seguintes:

Python
PyBoleto https://github.com/thiagosm/pyboleto parece ser a que tem a data mais recente de atualização,
dentro do repo erpbrasil onde esta a NFe foi criado o https://github.com/erpbrasil/erpbrasil.febraban que pela descrição
seria o PyBoleto mantido pela KMEE mas está como Arquivado, talvez isso pode ser reativado
Python-Boleto-CNAB https://github.com/marciorpbradoo/python-boleto-cnab esse parece ter a implementação do CNAB 240 Itau SISPAG mas a Data de Atulização tem 6 anos https://github.com/marciorpbradoo/python-boleto-cnab/tree/master/python-cnab/cnab240/bancos/itauSispag
Python-CNAB https://github.com/Trust-Code/python-cnab parece ser um Fork do PyBoleto mantido pela Trust-Code

PHP
https://github.com/QuilhaSoft/OpenCnabPHP API https://github.com/jersobh/php-boleto-cnab o CNAB 240 do Itau está
definido como Beta

NodeJS
https://github.com/banco-br/nodejs-cnab

Eu recomendaria buscar incluir no BRCobranca, acredito que o Kivanio não irá recusar um PR nesse sentido, vocês já atualizaram o Banco AILOS conhecem a estrutura tanto da API quanto dos módulos na OCA e na API podemos colocar um repo Fork enquanto o código não for integrado no repo principal, além de fortalecer essa biblioteca que já é usada dentro da OCA, mas essa decisão cabe a vocês, se seguirem nesse caminho existe um repo que parece ser um fork do BRCobranca feito a uns 7 anos atras mas que parece ter a estrutura e o código em ruby do SISPAG do Itau no 240 talvez ajude a ver como pode ser feito https://github.com/eduardordm/cnab240 .

cc @renatonlima @rvalyi @mileo @marcelsavegnago

@antoniospneto
Copy link
Contributor Author

antoniospneto commented Nov 8, 2022

Motivos que levaram a não utilizar uma lib externa:

  1. Não encontramos nenhuma lib pronta madura o suficiente para tratar os casos de CNAB Pagamentos (p/ terceiros)
  2. Optamos por fazer um configurador usando as visualizações Odoo, essa é a parte central, coração do módulo. Desta forma toda a configuração fica fácil de visualizar e organizar, é possível configurar vários bancos de uma forma simples, fácil de testar e sem escrever código.
  3. Por não precisar codificar as particularidades dos bancos ,o código do processador CNAB acaba ficando relativamente simples, achamos que não faz muito sentido extrair essa parte da lógica para uma camada externa, pois a dificuldade para fazer a integração acabaria superando a complexidade do próprio código.
  4. Nossa experiencia com o brCobranca. Ela é uma boa lib e sempre nos atendeu bem na parte de cobrança, porém para fazer qualquer alteração acaba sendo mais desgastante, pois é sempre preciso fazer alteração na camada Odoo e também na camada da Lib e as vezes a camada intermediaria do serviço da API. Cada uma tem suas particularidades, uma é em Python outra em Ruby e todas tem que ser testadas e debugadas de forma individual...
  5. Também referente a experiência do BRCobranca, o tratamento dos dados que estão sendo enviados e recebidos da lib é bastante denso e complexo, e o código para fazer essa parte acaba sendo maior que o próprio código do nosso processador CNAB (código python). Exemplos: Quantidade de ifs ou métodos para verificação do banco, os zfill de tratamento, tratamento de data, entre outros..
  6. O código do CNAB Structure não sobrescreve comportamento de outros módulos, quando acontece é só alguns pontos e de forma bem superficial, então não exige muito esforço para acompanhar as atualizações do Odoo. É um código fácil de se manter.

Possibilidade de juntar mais de um Payment Line (Transações) em uma Bank Line ( Linha Bancária):

Resgatamos esse comportamento que já é nativo do Payment Order para funcionar normalmente quando a ordem for para CNAB de pagamentos a terceiro, para a cobrança concordamos que isso não se aplica. Pense no caso que uma empresa tem mais de uma fatura de um determinado fornecedor vencendo no mesmo dia, é mais comodo e as vezes mais barato (evitar tarifa a mais) juntar o pagamento em uma transferia só.

No processamento do arquivo retorno:

  1. Optamos por ter essa etapa a mais, de primeiro visualizar o arquivo e depois processar, pois desta forma é possível conferir, validar e auditar o arquivo antes de confirmar o processamento que vai alterar o estado dos dados no sistema.. Caso ocorra qualquer erro enquanto o arquivo é processado é possível voltar para a etapa inicial e conferir os dados que estão no arquivo de retorno e entender as possíveis causa da falha. Hoje no BrCobrança isso não é possível.
  2. Outro comportamento que a gente quis fazer diferente, é que no brcobrança após processar o retorno, caso o arquivo tenha eventos de liquidação e eventos de notificação juntos, o sistema nos leva diretamente para a listagem dos lançamentos contábeis e os eventos de notificação acabam ficando escondidos para o usuário, apesar de que isso pode ser visto voltando para a tela do return log, preferimos trazer essas informações logo após a importação.
  3. Não concordamos que isso pode ser uma falha para a segurança, as informações na tela do retorno são apenas para leitura e não podem ser editadas. Até mesmo na parte de configuração da estrutura CNAB apenas usuários com permissão especifica podem fazer alterações.
  4. Apesar de ter mais cliques para o usuário final, o processo ainda é relativamente simples, as vantages na segurança das informações e a clareza no processo compensam.

Sobre o módulo l10n_br_account_payment_order

Não entendemos o que você acha que não entendemos referente ao l10n_br_account_payment_order, pois estudamos antes de fazer as alterações, cuidamos para dividir as responsabilidades, o que é de regra genérica envolvendo os padrões do sistema bancário brasileiro foram implementados neste módulo, ver as pr: #2218 #2180 #2141 #2123 #2090 #2081 #2147 #2139 #2138

Tracking e Audit-log:

Realmente achamos interessante, podemos considerar isso como uma sugestão de melhoria para o módulo.

@mbcosta obrigado por nos ajudar a debater as ideais, espero que a gente possa entrar em um consenso.

Aguardamos as revisões dos PSCs para darem a sua própria opinião.

Obs: a cobertura de testes ainda está 67%, vamos adicionar mais até chegar aos 85%.

@mbcosta
Copy link
Contributor

mbcosta commented Nov 12, 2022

@antoniospneto

  • "4. Nossa experiencia com o brCobranca. Ela é uma boa lib e sempre nos atendeu bem na parte de cobrança, porém para fazer qualquer alteração acaba sendo mais desgastante, pois é sempre preciso fazer alteração na camada Odoo e também na camada da Lib e as vezes a camada intermediaria do serviço da API. Cada uma tem suas particularidades, uma é em Python outra em Ruby e todas tem que ser testadas e debugadas de forma individual..."

Sim acredito que o README precisa ser atualizado sobre como debugar, não sei se todos fazem da mesma forma mas uma referencia mesmo básica de como pode ser debugada cada camada seria bom existir.

  • "5. Também referente a experiência do BRCobranca, o tratamento dos dados que estão sendo enviados e recebidos da lib é bastante denso e complexo, e o código para fazer essa parte acaba sendo maior que o próprio código do nosso processador CNAB (código python). Exemplos: Quantidade de ifs ou métodos para verificação do banco, os zfill de tratamento, tratamento de data, entre outros.."

Idealmente o BRCobranca deveria retornar os campos padronizados, cheguei a incluir um TODO
nesse sentido aqui https://github.com/OCA/l10n-brazil/blob/14.0/l10n_br_account_payment_brcobranca/parser/cnab_file_parser.py#L107 e na herança do modulo account_move_base_import, que foi uma especificação, foi preciso sobre escrever diversos métodos pois a forma que ele foi pensando não inclui o caso de processar um arquivo quando não gera account.move, é preciso fazer alterações nesse modulo para ter uma herança mais limpa( provavelmente alterações nesse modulo só serão aceitas em migrações ou na versão mais atual). Essas alterações devem diminuir um pouco o código e o ideal é ser o menor possível, tanto por convergências no l10n_br_account_payment_order, refatorações no account_move_base_import e padronização do BRCobranca.

  • "Pense no caso que uma empresa tem mais de uma fatura de um determinado fornecedor vencendo no mesmo dia, é mais comodo e as vezes mais barato (evitar tarifa a mais) juntar o pagamento em uma transferia só."

Se for espelho do arquivo não vejo problema, é preciso validar se a parametrização é suficiente para identificar quando juntar e quando não juntar.

  • "1. Optamos por ter essa etapa a mais, de primeiro visualizar o arquivo e depois processar, pois desta forma é possível conferir, validar e auditar o arquivo antes de confirmar o processamento que vai alterar o estado dos dados no sistema.. Caso ocorra qualquer erro enquanto o arquivo é processado é possível voltar para a etapa inicial e conferir os dados que estão no arquivo de retorno e entender as possíveis causa da falha. Hoje no BrCobrança isso não é possível."

Podemos ter convergência nessa questão, mas é importante entender o contexto sobre o que leva um usuário a auditar o arquivo antes de processar? Pensando no caso do CNAB já estar Homologado, eu diria que duplicidade, importar um arquivo já importado, ou quando se trabalha com vários Bancos, isso pode justificar, tem outras? Mas é bom considerar que isso também permite que alguém fique testando alterações nos arquivos até obter um resultado esperado e isso não tem registro, por isso a referencia a fraude, esse problema pode acontecer em ambas implementações é preciso ver se mostrar ou não os resultados é algo que dificulta esse tipo de problema.

  • "2. Outro comportamento que a gente quis fazer diferente, é que no brcobrança após processar o retorno, caso o arquivo tenha eventos de liquidação e eventos de notificação juntos, o sistema nos leva diretamente para a listagem dos lançamentos contábeis e os eventos de notificação acabam ficando escondidos para o usuário, apesar de que isso pode ser visto voltando para a tela do return log, preferimos trazer essas informações logo após a importação."

Isso pode ser revisto, acredito que quando fiz queria apenas confirmar que tinha sido feita a reconciliação, aqui a opinião do que o usuário espera talvez seja o que vai definir, ou mesmo um parâmetro se necessário

  • "3. Não concordamos que isso pode ser uma falha para a segurança, as informações na tela do retorno são apenas para leitura e não podem ser editadas. Até mesmo na parte de configuração da estrutura CNAB apenas usuários com permissão especifica podem fazer alterações."

Se estão somente Leitura/readonly não vejo problema, a questão é a que escrevi acima de permitir alguém ficar testando arquivos e vendo os resultados, ou se isso não é um problema? É sempre necessário homologar junto ao banco antes de iniciar o uso, depois disso deveria funcionar sem problemas, mas podem existir motivos que justifiquem ou podemos pensar se deveriam existir as duas formas de importação? Uma para homologar e outra para o dia a dia, deixar essa decisão via parâmetro ou no Banco, ou no Diario ou no Modo de Pagto um "dia/dia" outro "auditoria"? Um Roadmap do BRCobranca e enviar no JSON de retorno a informação do Número Sequencial do Arquivo e a Data do Arquivo para registrar e usar esses campos para informar o usuário sobre uma possível duplicidade ou mesmo bloquear a importação se for o caso. Sobre a estrutura do CNAB é a necessidade de além da permissão de acesso é preciso registrar alterações para ter rastreabilidade, exemplo um cenário teórico dois usuários tem permissão de alteração, existe uma alteração que precisa ser investigada, no modulo pelo o que vi vai ser preciso entrar no Banco de Dados e ver o usuário e Data do último Write, isso não informa o que foi alterado, se existirem alterações dos dois usuários em datas próximas não vai ser possível dizer quem fez o que, o que é um problema que vai recair sobre o responsável/desenvolvedor por isso no caso CNAB esse LOG é importante para proteger os desenvolvedores.

  • "Não entendemos o que você acha que não entendemos referente ao l10n_br_account_payment_order, pois estudamos antes de fazer as alterações, cuidamos para dividir as responsabilidades, o que é de regra genérica envolvendo os padrões do sistema bancário brasileiro foram implementados neste módulo, ver as pr"

Acredito que entenderam boa parte, o que falo é que teve um debate que ficou restrito a poucas pessoas e precisa ter uma referencia sobre no README do l10n_br_acccount_payment_order como um aviso do tipo:

"IMPORTANTE: Caso se queira implementar ou expandir o CNAB procure evitar a ideia de incluir uma Biblioteca CNAB dentro de um modulo na Localização, isso foi proposto na versão 8.0 https://github.com/odoo-brazil/odoo-brazil-banking/tree/8.0 e durante o encontro da comunidade em 2017 houve um debate sobre e o consenso foi que o melhor é implementar com uma biblioteca externa e que se possa integrar mais de uma( a Localização não deve obrigar o uso de uma Biblioteca especifica ), porque idealmente no repositório da Localização deve, quando possível, existir apenas o código que localiza o Brasil e que varia de acordo com as mudanças de versões do Odoo, tudo o que não varia com as versões e puder ser extraído ou implementado em outros repositórios ou em bibliotecas externas será melhor, isso atende os conceitos de modularidade e abstração e vai desde de códigos simples como "Transformar Números em Palavras", "Formatação/Validação de CPF/CNPJ, IE, PIS", "Remoção de pontuação de documentos" ou complexas como pesquisa de CEP e a geração, transmissão e assinatura de Notas Fiscais Eletrônicas, o recomendado é procurar integrar com as bibliotecas já existentes ou implementar outra ou criar uma."

Pelo o que entendo caso a ideia de ter uma Biblioteca CNAB dentro Localização não fosse uma questão a ser evitada provavelmente o código do odoo-brazil-banking estaria na OCA desde da versão 8.0 e é possível ver que os PRs sobre CNAB que foram feitos no sentido de fazer uma integração com alguma biblioteca externa.

Então as questões seriam existe semelhança desse PR com a proposta que havia sido feita no odoo-brazil-banking? Basicamente saber se ambos querem incluir uma Biblioteca CNAB dentro de um modulo da localização? E se essa especificação de usar Bibliotecas Externas para o CNAB deve ser reconsiderada dentro da OCA?

Essa ideia de modularizar e abstrair pode ser aplicada ao PR proposto, por exemplo podemos pensar se seria possível dividir em dois módulos o l10n_br_cnab_structure para tentar separar o código que varia de acordo com cada versão do odoo do que não varia, exemplo:
l10n_br_cnab_structure_1 a base onde está a estrutura do CNAB, o código que não varia de acordo com as versões do odoo
l10n_br_cnab_structure_2 o que liga o primeiro ao Odoo e que varia de acordo com as versões do odoo

image

Bem superficial talvez tenha detalhes a verificar, mas a pergunta é se é possível fazer essa separação? E se sim isso também é importante para o modulo poque o 1 uma vez estabilizado provavelmente não terá alterações, somente adições de arquivos CSV mapeando outros CNABs, enquanto o 2 muda com as versões do odoo, isso ajudaria a gestão da implementação e dos módulos em si, e a partir daí perguntar se o código não varia de acordo com as versões odoo não seria melhor ele ser extraído?

Acredito que podemos chegar em muitos consensos e a arquitetura permite a existência da divergência.

@marcelsavegnago
Copy link
Member

marcelsavegnago commented Nov 14, 2022

Podemos ter convergência nessa questão, mas é importante entender o contexto sobre o que leva um usuário a auditar o arquivo antes de processar? Pensando no caso do CNAB já estar Homologado, eu diria que duplicidade, importar um arquivo já importado, ou quando se trabalha com vários Bancos, isso pode justificar, tem outras? Mas é bom considerar que isso também permite que alguém fique testando alterações nos arquivos até obter um resultado esperado e isso não tem registro, por isso a referencia a fraude, esse problema pode acontecer em ambas implementações é preciso ver se mostrar ou não os resultados é algo que dificulta esse tipo de problema.

@mbcosta Entendo que estamos falando da tela que apresenta como foi o processamento do que foi enviado paga o banco... certo? Eu entendo que um usuário pode sim se valer desta tela para ficar manipulando e validando envios e entendo o risco. Porém, operacionalmente entendo que faz sentido para quem é responsável pelo processo ter este tipo de visão referente ao retorno.

@marcelsavegnago
Copy link
Member

Se estão somente Leitura/readonly não vejo problema, a questão é a que escrevi acima de permitir alguém ficar testando arquivos e vendo os resultados, ou se isso não é um problema? É sempre necessário homologar junto ao banco antes de iniciar o uso, depois disso deveria funcionar sem problemas, mas podem existir motivos que justifiquem ou podemos pensar se deveriam existir as duas formas de importação? Uma para homologar e outra para o dia a dia, deixar essa decisão via parâmetro ou no Banco, ou no Diario ou no Modo de Pagto um "dia/dia" outro "auditoria"? Um Roadmap do BRCobranca e enviar no JSON de retorno a informação do Número Sequencial do Arquivo e a Data do Arquivo para registrar e usar esses campos para informar o usuário sobre uma possível duplicidade ou mesmo bloquear a importação se for o caso. Sobre a estrutura do CNAB é a necessidade de além da permissão de acesso é preciso registrar alterações para ter rastreabilidade, exemplo um cenário teórico dois usuários tem permissão de alteração, existe uma alteração que precisa ser investigada, no modulo pelo o que vi vai ser preciso entrar no Banco de Dados e ver o usuário e Data do último Write, isso não informa o que foi alterado, se existirem alterações dos dois usuários em datas próximas não vai ser possível dizer quem fez o que, o que é um problema que vai recair sobre o responsável/desenvolvedor por isso no caso CNAB esse LOG é importante para proteger os desenvolvedores.

@mbcosta

Entendo que a integração foi homologada com o banco mas no dia a dia é complicado garantir que operacionalmente não será feita alguma coisa errada que pode gerar algum tipo de recusa pelo banco. Acredito que esta visão do log (seja onde for ou mesmo até com algum tipo de permissão a mais para poder visualizar) ajuda bastante no dia a dia do operacional.

Em relação ao LOG acredito que através do audit log isto seria atendido e apenas para não obrigar o usuário talvez seja interessante incluir no README orientação sobre o uso do mesmo afim de conseguir rastrear as alterações.. oq acha ?

@marcelsavegnago
Copy link
Member

marcelsavegnago commented Nov 14, 2022

Acredito que entenderam boa parte, o que falo é que teve um debate que ficou restrito a poucas pessoas e precisa ter uma referencia sobre no README do l10n_br_acccount_payment_order como um aviso do tipo:

"IMPORTANTE: Caso se queira implementar ou expandir o CNAB procure evitar a ideia de incluir uma Biblioteca CNAB dentro de um modulo na Localização, isso foi proposto na versão 8.0 https://github.com/odoo-brazil/odoo-brazil-banking/tree/8.0 e durante o encontro da comunidade em 2017 houve um debate sobre e o consenso foi que o melhor é implementar com uma biblioteca externa e que se possa integrar mais de uma( a Localização não deve obrigar o uso de uma Biblioteca especifica ), porque idealmente no repositório da Localização deve, quando possível, existir apenas o código que localiza o Brasil e que varia de acordo com as mudanças de versões do Odoo, tudo o que não varia com as versões e puder ser extraído ou implementado em outros repositórios ou em bibliotecas externas será melhor, isso atende os conceitos de modularidade e abstração e vai desde de códigos simples como "Transformar Números em Palavras", "Formatação/Validação de CPF/CNPJ, IE, PIS", "Remoção de pontuação de documentos" ou complexas como pesquisa de CEP e a geração, transmissão e assinatura de Notas Fiscais Eletrônicas, o recomendado é procurar integrar com as bibliotecas já existentes ou implementar outra ou criar uma."

@mbcosta
Entendo o ponto mas não apenas sobre o CNAB, alias, acho a API do brcobranca muito boa, mas acho que a percepção dos membros pode ter mudado um pouco de 2017 pra cá sobre o uso de bibliotecas externas para atender algumas necessidades. Digo isso principalmente ao considerar bibliotecas existentes que malemá são mantidas (nao eh o caso do brcobranca) e até mesmo alguns entraves com as bibliotecas já utilizadas que as vezes tem gerado dificuldade principalmente para entender que fork deve ser usado no momento. Posso estar errado.

Entendo também que ter uma biblioteca X que nao seja especifica para Odoo pode atrair mais pessoas.. mas fico com o pé atrás quando parte importante não está no controle da OCA.

cc @rvalyi @antoniospneto @felipemotter

@marcelsavegnago
Copy link
Member

Bem superficial talvez tenha detalhes a verificar, mas a pergunta é se é possível fazer essa separação? E se sim isso também é importante para o modulo poque o 1 uma vez estabilizado provavelmente não terá alterações, somente adições de arquivos CSV mapeando outros CNABs, enquanto o 2 muda com as versões do odoo, isso ajudaria a gestão da implementação e dos módulos em si, e a partir daí perguntar se o código não varia de acordo com as versões odoo não seria melhor ele ser extraído?

Sobre a modularização sempre vejo de forma positiva ❤️ . Então cabe mais um analise técnica mesmo.

@mbcosta
Copy link
Contributor

mbcosta commented Nov 14, 2022

@marcelsavegnago

-"Entendo que estamos falando da tela que apresenta como foi o processamento do que foi enviado paga o banco... certo? Eu entendo que um usuário pode sim se valer desta tela para ficar manipulando e validando envios e entendo o risco. Porém, operacionalmente entendo que faz sentido para quem é responsável pelo processo ter este tipo de visão referente ao retorno."

Falo do wizard que processa o Arquivo de Retorno, o arquivo que é gerado pelo Banco, sobre o arquivo a ser Enviado gerado pelo Odoo ou Biblioteca X hoje não vejo um problema, teria? Porque vai existir o arquivo original no programa para conferencia no caso de investigação sobre divergência, acredito que hoje não há uma forma de substitui-lo e ele é baseado nas informações da Ordem de Pagto/Debito, no caso especifico do l10n_br_cnab_structure é preciso ver se alguém mal intencionado pode alterar a especificação do CNAB e depois de gerar o Arquivo voltar a especificação do CNAB.

  • "Entendo que a integração foi homologada com o banco mas no dia a dia é complicado garantir que operacionalmente não será feita alguma coisa errada que pode gerar algum tipo de recusa pelo banco. Acredito que esta visão do log (seja onde for ou mesmo até com algum tipo de permissão a mais para poder visualizar) ajuda bastante no dia a dia do operacional."

Seria importante saber, como exemplos, esses casos "que pode gerar algum tipo de recusa pelo banco", porque no arquivo gerado pelo Odoo os campos precisam seguir a estrutura pre-definida do CNAB escolhido, e a conferencia disso pode ser feita verificando a posição dos campos no arquivo gerado e antes de iniciar o uso a homologação do Banco, depois disso teria algo a ser visto?

Sobre o Retorno/arquivo gerado pelo Banco hoje ou é apenas registrado o que veio ou em caso de Liquidações é feita a Reconciliação, mas podem vir outras informações que dependendo precisam ser tratadas, por isso também a importância em conhecer esses casos e ver se o programa deve fazer algo que hoje não é feito( Caso de Instrução não Aceita https://github.com/OCA/l10n-brazil/blob/14.0/l10n_br_account_payment_brcobranca/parser/cnab_file_parser.py#L393 tem um TODO para se definir se precisa ser feito algo).

É preciso diferenciar o responsável pela implementação alguém do TI Interno ou Terceiro do Usuário Final, são usos e responsabilidades diferentes, o usuário final até onde sei dificilmente vai ficar testando e validando arquivos mas quem estiver implementando sim.

A ideia geral seria ter um único Wizard de Importação do Arquivo Gerado pelo Banco no l10n_br_account_payment_order, mas uma das especificações no l10n_br_account_payment_brcobranca era usar o account_move_base_import para isso, como essa dependência não era uma convergência isso ficou apenas no brcobranca, eu acho importante debater para buscar um comportamento padrão, basicamente precisamos saber:

Importar o arquivo de uma vez ao invés de dois passos protege algo? Ou evita alguém ficar testando alterações? Na implementação isso pode acontecer e faz parte mas no Dia a Dia isso não seria suspeito? Se é um problema ter duas formas de Importar uma com dois passos e outra "de uma vez" seria o melhor? E se não protege nada podemos padronizar com os dois passos? Hoje pelo o que entendo o usuário apenas olharia se o arquivo não é duplicado e se é de um Banco especifico, fora isso teria algo a mais?

  • "Em relação ao LOG acredito que através do audit log isto seria atendido e apenas para não obrigar o usuário talvez seja interessante incluir no README orientação sobre o uso do mesmo afim de conseguir rastrear as alterações.. oq acha ?"

A recomendação do auditlog foi porque devido a quantidade de campos e forma do modulo eu acredito que o parâmetro Tracking é o mail.thread podem não dar conta ou "poluir" muito a visão, e tendo esse modulo como dependência se poderia incluir Arquivos de Dados/Data já configurando os objetos e campos necessários para monitorar dentro do l10n_br_cnab_structure, assim ao instalar o modulo isso já estaria configurado.

A questão de Fraudes é complexa por um aspecto que é difícil saber antes de descoberta como ela pode ser feita, o que tentamos é buscar identificar o que pode ser alterado que permitiria algo nesse sentido, reduzir as chances de ser feito e registrar alterações em campos importantes, a Licença de certa forma nos protege contra acusações mas é importante deixar claro a quem esta implementando sobre o problema, no caso do CNAB por envolver uma comunicação com Bancos eu recomendaria sempre ter algo nesse sentido, em nome do sono tranquilo.

  • "Entendo o ponto mas não apenas sobre o CNAB, alias, acho a API do brcobranca muito boa, mas acho que a percepção dos membros pode ter mudado um pouco de 2017 pra cá sobre o uso de bibliotecas externas para atender algumas necessidades. Digo isso principalmente ao considerar bibliotecas existentes que malemá são mantidas (nao eh o caso do brcobranca) e até mesmo alguns entraves com as bibliotecas já utilizadas que as vezes tem gerado dificuldade principalmente para entender que fork deve ser usado no momento. Posso estar errado.

Entendo também que ter uma biblioteca X que nao seja especifica para Odoo pode atrair mais pessoas.. mas fico com o pé atrás quando parte importante não está no controle da OCA.
"

O dialogo é fundamental, é impossível ver ou resolver um problema complexo com apenas uma visão, quanto mais casos de uso/cenários conhecemos, entendemos e atendemos melhor e mais robusta fica a implementação.

Sobre a mudança de percepção acho difícil, o debate é valido, mas isso é algo geral nos repositórios da OCA, se for feito no caso CNAB será uma exceção que dependendo dos argumentos pode se justificar, mas a tendencia geral parece ser que sempre que for possível usar uma biblioteca externa, extrair algo da localização, implementar algo na localização com partes em outros repositórios da OCA é isso que acabará sendo feito inicialmente ou a posterior.

Mesmo no caso de "bibliotecas existentes que malemá são mantidas" a opção pelo o que acompanho é a de que a comunidade do Odoo Brasil ou assume essas Bibliotecas ou faz um Fork delas ou cria novas, o https://github.com/erpbrasil pode ser visto como um exemplo desse esforço da comunidade mesmo composta por desenvolvedores odoo estarem fazendo bibliotecas genéricas.

"bibliotecas já utilizadas que as vezes tem gerado dificuldade principalmente para entender que fork deve ser usado no momento"

Existe um esforço em "limpar" das dependências as Bibliotecas com problemas, desatualizadas, abandonadas, defasadas e etc, mas isso não é tão simples quanto se gostaria, é preciso testes e validações que demandam muitas horas, nos últimos anos houve um foco na Localização na substituição do PySPED pelo NFeLIB e as bibliotecas relacionadas, não sei se você se refere a um período onde essa transição estava com essas questões de se usar Forks, hoje acredito que está mais estabilizado, mas é importante identificar bibliotecas com problemas ou que devem ser substituídas até mesmo com um issue dentro da localização, como isso não acontece de maneira rápida é bom documentar para outros estarem atentos ou mesmo alguém ou uma empresa incluírem em um orçamento uma quantidade de horas a mais para se dedicar a solução do problema.

P.S: Acabei clicando sem querer no botão e subi um rascunho desse cometário antes de finalizar, por isso a edição

@marcelsavegnago
Copy link
Member

marcelsavegnago commented Nov 16, 2022

Falo do wizard que processa o Arquivo de Retorno, o arquivo que é gerado pelo Banco, sobre o arquivo a ser Enviado gerado pelo Odoo ou Biblioteca X hoje não vejo um problema, teria? Porque vai existir o arquivo original no programa para conferencia no caso de investigação sobre divergência, acredito que hoje não há uma forma de substitui-lo e ele é baseado nas informações da Ordem de Pagto/Debito, no caso especifico do l10n_br_cnab_structure é preciso ver se alguém mal intencionado pode alterar a especificação do CNAB e depois de gerar o Arquivo voltar a especificação do CNAB.

Falo do wizard de importação do retorno. No envio concordo que o que está na ordem de pagament serve como parametro para conferência.

Seria importante saber, como exemplos, esses casos "que pode gerar algum tipo de recusa pelo banco", porque no arquivo gerado pelo Odoo os campos precisam seguir a estrutura pre-definida do CNAB escolhido, e a conferencia disso pode ser feita verificando a posição dos campos no arquivo gerado e antes de iniciar o uso a homologação do Banco, depois disso teria algo a ser visto?

Sinceramente não sei dizer quais poderiam ser os casos de recusa (sem falar de layout).. Talvez algum boleto que não possa ser criado para o cpf/cnpj X, um dado especifico que nao seja valido.. Meu receio é o cliente achar que algo foi processado e por algum motivo não foi. Porém, sinceramente não sei dizer se aconteceria este tipo de problema e com que frequencia poderia ocorrer algum problema mas de qq forma uma visão geral para quem é responsável pelo envio e pela garantia do processamento poderia ser interesse.

A recomendação do auditlog foi porque devido a quantidade de campos e forma do modulo eu acredito que o parâmetro Tracking é o mail.thread podem não dar conta ou "poluir" muito a visão, e tendo esse modulo como dependência se poderia incluir Arquivos de Dados/Data já configurando os objetos e campos necessários para monitorar dentro do l10n_br_cnab_structure, assim ao instalar o modulo isso já estaria configurado.

Concordo que o auditlog tem mais condições de fazer o trabalho que precisa. De imediato nesta PR eu incluiria alguma orientação no readme e em um segundo momento poderia ser implementando os dados necessários para criar a auditoria.

A questão de Fraudes é complexa por um aspecto que é difícil saber antes de descoberta como ela pode ser feita, o que tentamos é buscar identificar o que pode ser alterado que permitiria algo nesse sentido, reduzir as chances de ser feito e registrar alterações em campos importantes, a Licença de certa forma nos protege contra acusações mas é importante deixar claro a quem esta implementando sobre o problema, no caso do CNAB por envolver uma comunicação com Bancos eu recomendaria sempre ter algo nesse sentido, em nome do sono tranquilo.

De fato... podemos mitigar ao máximo mas é complicado mesmo.

O dialogo é fundamental, é impossível ver ou resolver um problema complexo com apenas uma visão, quanto mais casos de uso/cenários conhecemos, entendemos e atendemos melhor e mais robusta fica a implementação.

Fato.

Sobre a mudança de percepção acho difícil, o debate é valido, mas isso é algo geral nos repositórios da OCA, se for feito no caso CNAB será uma exceção que dependendo dos argumentos pode se justificar, mas a tendencia geral parece ser que sempre que for possível usar uma biblioteca externa, extrair algo da localização, implementar algo na localização com partes em outros repositórios da OCA é isso que acabará sendo feito inicialmente ou a posterior.

Falando das partes distribuidas nos repositórios da OCA.. concordo totalmente. O ideal é que cada coisa esteja no seu devido logal. Algum tempo atrás questionei sobre o módulo payment_pagseguro por entender que é um método de pagamento e portanto deveria estar em outro repositório. Na época acabou ficando provisóriamente no repo da localização mas em breve deve acaber sendo movido para um repositório mais adequado já que nada impede do método de pagamento pagseguro ser utilizado por outros paises. No caso do CNAB de fato podemos pensar da mesma forma mas não sei se caberia em outro REPO da OCA.

Mesmo no caso de "bibliotecas existentes que malemá são mantidas" a opção pelo o que acompanho é a de que a comunidade do Odoo Brasil ou assume essas Bibliotecas ou faz um Fork delas ou cria novas, o https://github.com/erpbrasil pode ser visto como um exemplo desse esforço da comunidade mesmo composta por desenvolvedores odoo estarem fazendo bibliotecas genéricas.

Como comentei antes, entendo que esta abstração do Odoo é valida principalmente por conta de poder contar até mesmo com outros desenvolvedores que não sejam Odoo. Não sei dizer se caberia no caso desta PR mas cabe avaliar. Se for algo pequeno não sei se justificaria o esforço de colocar e manter fora da OCA. Acredito que vc, o @antoniospneto e o @felipemotter podem falar melhor sobre isso.

Existe um esforço em "limpar" das dependências as Bibliotecas com problemas, desatualizadas, abandonadas, defasadas e etc, mas isso não é tão simples quanto se gostaria, é preciso testes e validações que demandam muitas horas, nos últimos anos houve um foco na Localização na substituição do PySPED pelo NFeLIB e as bibliotecas relacionadas, não sei se você se refere a um período onde essa transição estava com essas questões de se usar Forks, hoje acredito que está mais estabilizado, mas é importante identificar bibliotecas com problemas ou que devem ser substituídas até mesmo com um issue dentro da localização, como isso não acontece de maneira rápida é bom documentar para outros estarem atentos ou mesmo alguém ou uma empresa incluírem em um orçamento uma quantidade de horas a mais para se dedicar a solução do problema.

Sim, este esforço tem acontecido e de fato está se estabilizando. Enfim, meu receio está atrelado ao cuidado em ter dependencias importantes fora do controle da OCA já que mesmo dentro da OCA as divergências existem mas de uma forma ou de outra prevalece existem recursos de critéros claros na OCA em relação a forma como tratar as discordancias. Já fora da OCA fica bem mais dificil.

@mbcosta entendo bem seu ponto de vista e agradeço muito a ajuda e conselhos que tem dado. Acredito que se for de interesse de todos é válido uma call entre os principais envolvidos. Como participantes eu sugiro:

@mbcosta acho que nao tenho vc no telegram.. se puder me adicionar eu agradeço @marcelsavegnago ... (fica frio que nao pretendo te incomodar praticamente nunca :D e sou da politica do nohello https://nohello.net/en/

@rvalyi
Copy link
Member

rvalyi commented Nov 16, 2022

pessoal, eu ainda não li as ultimas considerações, mas eu queria dizer que eu tinha visto esse modulo tb um tempo atras https://github.com/OCA/bank-payment/tree/14.0/account_payment_order_return, sera se não serviria pro CNAB tb? Não pensei mesmo, so informando da existência desse modulo mesmo...

@antoniospneto
Copy link
Contributor Author

antoniospneto commented Nov 17, 2022

pessoal, eu ainda não li as ultimas considerações, mas eu queria dizer que eu tinha visto esse modulo tb um tempo atras https://github.com/OCA/bank-payment/tree/14.0/account_payment_order_return, sera se não serviria pro CNAB tb? Não pensei mesmo, so informando da existência desse modulo mesmo...

@rvalyi

Não tinha percebido antes, dei uma olhada agora no account_payment_return e isso se encaixa muito bem, realmente acho que teria sido bem melhor se aproveitar dele, a estrutura de módulos a baixo poderia ter sido usado:

https://github.com/OCA/account-payment/tree/14.0/account_payment_return
Implementa o que seria o "return log" aqui da localização

https://github.com/OCA/account-payment/tree/14.0/account_payment_return_import
implementa um wizard genérico para a importação do arquivo de retorno.

https://github.com/OCA/bank-payment/tree/14.0/account_payment_order_return
Integra o return com o order.

Porém agora para fazer esse refactor pode levar um tempo, mas acho muito importante por isso no road-map do módulo para que no futuro isso seja feito e assim possamos manter o módulo mais consistente com o ecossistema da OCA.
Você acha que é tranquilo manter o módulo da forma que está incluir na localização assim e no futuro fazer essas alterações?

https://github.com/OCA/account-payment/tree/14.0/account_payment_return_import_iso20022
** exemplo de uma implementação final, bom pra testar a funcionalidade desses módulos.

@rvalyi
Copy link
Member

rvalyi commented Nov 18, 2022

Sim eu acho tranquilo ter esse primeiro passo como tá hoje e depois fazer um refator. Eu TB acho que esse payment_return começou a pintar na OCA depois que a gente já tinha bolado um forma de lidar com os retornos. Eu ia falar pro @mbcosta uns 2 anos atrás já, mas na correria não deu para ver ainda.
Mas enfim agora com vcs mexendo nisso TB e comprometidos com a qualidade do projeto eu imagino que na frente a gente consegue encarar esse refator.

@antoniospneto antoniospneto force-pushed the 14.0-add-cnab-structure-without-payment-way branch 2 times, most recently from a723afc to b068505 Compare January 21, 2023 17:23
@marcelsavegnago
Copy link
Member

@antoniospneto acho que pode deixar esta PR pronta para Review.

@marcelsavegnago
Copy link
Member

Testes funcionais e revisão Ok

@antoniospneto antoniospneto force-pushed the 14.0-add-cnab-structure-without-payment-way branch from b068505 to 3315af6 Compare January 21, 2023 22:22
@rvalyi
Copy link
Member

rvalyi commented Jan 21, 2023

@antoniospneto @felipemotter @marcelsavegnago quase OK para mim, obrigado pelo esforço com os testes. So uma coisa, a gente tinha comentado que mesmo se a gente deixasse o conceito do payment.way talvez o nome não era o mais adequado... Que tal renomear como payment type? Parece que seria isso não https://www.investopedia.com/terms/p/payment.asp ?

@antoniospneto
Copy link
Contributor Author

antoniospneto commented Jan 21, 2023

@rvalyi

Esse modelo que ficou é o "cnab.payment.way" o escopo dele é limitado para o uso do CNAB, é pra cadastrar as formas de pagamentos conforme está no manual do integração CNAB do banco. É diferente do modelo/conceito de payment.way que tava em discussão antes, esse sim foi decido ser removido, não está incluso nessa pr. Mas se ainda estiver estranho o nome podemos alterar, só não queria muito pois na hora de pesquisar no código vai confundir bastante com o payment_type nativo.

Faltou um pouquinho de testes pra chegar nos 86,18% do target dos coverage 😄 tudo bem ficar assim 85.77% ?

O módulo ainda teem muito pontos que podem ser melhorados, mas ele já cumpre o que promete.
Com o merge vamos poder discutir as melhorarias e integrar as novas funcionalidades aos poucos, também fica mais fácil do @douglascstd e @marcelsavegnago colaborar com as implementações das novas formas de pagamento que está em andamento.

Agradeço a todos que colaboraram com as dicas, discussões e revisões.

A PR está pronta para a revisão final :)

@antoniospneto antoniospneto marked this pull request as ready for review January 21, 2023 23:17
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.

OK, código aprovado. Parabens pelo grande trabalho @antoniospneto @felipemotter isso é mais um pedaço bem importante que esta sendo acrescentado na localização!

@rvalyi
Copy link
Member

rvalyi commented Jan 22, 2023

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 14.0-ocabot-merge-pr-2181-by-rvalyi-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Jan 22, 2023
Signed-off-by rvalyi
@OCA-git-bot
Copy link
Contributor

@rvalyi your merge command was aborted due to failed check(s), which you can inspect on this commit of 14.0-ocabot-merge-pr-2181-by-rvalyi-bump-nobump.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@rvalyi
Copy link
Member

rvalyi commented Jan 22, 2023

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

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

@OCA-git-bot OCA-git-bot merged commit 493334e into OCA:14.0 Jan 22, 2023
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 77c2c49. 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.

7 participants