[14.0][REF] l10n_br_account_payment_order, l10n_br_account_payment_brcobranca: Separando a Configuração do CNAB do Modo de Pagamento #3243
Conversation
|
Hi @rvalyi, |
l10n_br_account_payment_order/views/l10n_br_cnab_config_view.xml
Outdated
Show resolved
Hide resolved
| class L10nBrCNABCode(models.Model): | ||
| _name = "l10n_br_cnab.code" | ||
| _inherit = "mail.thread" | ||
| _description = "CNAB Code" |
There was a problem hiding this comment.
CNAB Code, não me parece um nome bom, poderia ser mais especifico, já que é um modelo para guardar a opção de valor que pode ser preenchido em um campo cnab, sugiro mudar o nome para "CNAB Field Option"
minha sugestão:
nome do arquivo: l10n_br_cnab_field_option.py
_name: l10n_br.cnab.field.option
_description = "CNAB Field Option""
o campo code_type podia mudar para field_name
There was a problem hiding this comment.
Colocar nomes em objetos ou campos pode ser um problema, o que faço para tentar descobrir qual é o melhor nome e de forma genérica tentar responder
"Qual o objetivo ou função do objeto ou campo?"
Nesse caso esse objeto é usados para armazenar, cadastrar os "Códigos do CNAB", cada CNAB tem um conjunto de Códigos, a maioria das Documentações que vi se referem a esses dados como Códigos, exemplo "Código de Instrução do Movimento", "Código de Retorno do Movimento", "Código de Alteração do Vencimento", "Código de Alteração do Valor do Título", etc no Arquivo de Remessa não é uma Opção não preencher o "Código de Instrução do Movimento" e cada código tem um objetivo
O campo code_type deriva da explicação acima porque os Códigos são agrupados em conjuntos os de Instrução não são iguais aos de Retorno e até possuem o mesmo valor no campo code exemplo o 01 pode ser tanto uma Instrução quanto um Retorno, o que acredito que difere entre eles, e o termo que que encontrei, é o Tipo existem o Códigos CNAB do Tipo Instrução de Movimento, do Tipo Retorno de Movimento, do Tipo Código da Carteira, do Tipo Código de Desconto e etc não vejo relação com um field_name/ Nome do Campo o objeto já possui um campo name
|
No geral me parece uma boa mudança, deixei umas observações a cima que eu acho que pode ajudar a deixar mais claro. |
Muito bom! Vai ajudar a mantermos dados de configuração para as carteiras! Reviso em breve. |
mbcosta
force-pushed
the
14.0-REF-unify_cnab_code_separate_cnab_from_payment_mode
branch
from
9223764
to
ce32888
Compare
|
Force-push para atualizar o README dos módulos, alguns pontos:
|
|
@mbcosta rola um rebase por favor? |
mbcosta
force-pushed
the
14.0-REF-unify_cnab_code_separate_cnab_from_payment_mode
branch
from
ce32888
to
83f01b0
Compare
|
@mbcosta já da pra revisar? Ou vc pretende mudar mais coisa? |
mbcosta
force-pushed
the
14.0-REF-unify_cnab_code_separate_cnab_from_payment_mode
branch
2 times, most recently
from
323eb35
to
c8e3a1c
Compare
|
valeu @rvalyi @mileo , fiz um Force-Push para fazer o rebase ontem e acabei resolvendo um conflito de arquivo errado e acabou gerando erro nos testes, já corrigido. Por enquanto a única alteração que estou considerando é separar as alterações nos README em um commit específico, talvez isso seja melhor para revisão, o que acham, seria melhor? Para quem for fazer a Revisão eu recomendo ao invés de ir direto na aba Arquivos Alterado/Files changed clicar nos Commits para ver por partes, uma breve descrição dos commits: 1 - [REF] l10n_br_account_payment_order: Unified CNAB Codes in only one object and separated CNAB Configuration from Payment Mode Aqui estão as principais alterações, uma forma de visualizar a separação do Modo de Pagamento das Configurações CNAB é olhar como ficaram os Dados de Demonstração do Modo de Pagamento e do novo arquivo CNAB Config, o que reflete no Banco de Dados, ou também pelo README 2 - [REF] l10n_br_account_payment_order: Unified CNAB Codes Data in only one object O que gera o maior DIFF porque ao unificar os Códigos CNAB é preciso alterar nos arquivos DATA esse novo objeto e incluir o campo Tipo do Código, o que permite unir os campos, então esse commit é repetitivo é aqui os erros que podem existir são de Digitação, algum nome ou valor de campo errado, nesses casos pode ocorrer Warnings ou mesmo ERROR no LOG. 3 - [REF] l10n_br_account_payment_brcobranca: Unified CNAB Codes in only one object and separated CNAB Configuration from Payment Mode As alterações necessárias no modulo do BRCobranca para usar o CNAB Config para obter os dados do CNAB e não mas diretamente do Modo de Pagamento e ao consultar algum Código CNAB usar o novo objeto l10n_br_cnab.code e incluir o Tipo do Código se é uma Instrução de Movimento, Retorno do Movimento ou Código de Carteira (por enquanto os possíveis códigos). Infelizmente não vejo uma forma de extrair essas alterações em PRs menores para facilitar a Revisão sem gerar erro nos testes do CI. O LOG passou a retornar WARNINGS de campos com nomes iguais e a Cobertura dos Testes diminui com esse PR, porém como escrevi antes isso é temporário e foi necessário manter para rodar os Scripts de Migração, mas também garante que esses Dados podem ser consultados eventualmente em caso de algum problema depois do Merge, acredito que depois de alterar os Códigos de Desconto, Multa, ????? de CHAR para Objetos já poderá ser feito um PR de "limpeza" removendo Campos, Visões e Objetos o que vai resolver esse Warnings no LOG e melhorando a Cobertura dos Testes. |
mbcosta
force-pushed
the
14.0-REF-unify_cnab_code_separate_cnab_from_payment_mode
branch
2 times, most recently
from
4e2d07b
to
b175eb8
Compare
|
Com a extração das alterações referentes a Unificar os Códigos CNAB esse PR passou a depender do PR #3337 eu incluí aqui os commits para evitar erros mas assim que houver o merge eu vou ver de fazer o rebase e aqui deverá ter apenas os commits referentes a Separar a Configuração CNAB do Modo de Pagamento. Com essa alteração a Migração também foi dividida:
Aproveitei e também dividi os commits para facilitar a revisão:
Removi as alterações do README para diminuir o DIFF, pretendo fazer em outro PR. Peço desculpas para quem eu havia falado que o PR estava pronto para Revisão antes, @rvalyi @mileo , porque ao separar vi que alguns Códigos do CNAB do Bradesco e um do AILOS estavam sem informar o novo campo code_type o que identifica se é um Código de Instrução ou Retorno ou Carteira, acredito que agora está tudo certo e que devido a quantidade de Aprovações no PR #3337 esse PR agora passa apenas a aguardar o merge mas já é possível fazer a Revisão. |
mbcosta
force-pushed
the
14.0-REF-unify_cnab_code_separate_cnab_from_payment_mode
branch
3 times, most recently
from
6fdf2e2
to
18b3e72
Compare
mbcosta
changed the title
|
Apenas para registrar que com o merge do PR #3337 aqui passou a conter apenas a Separação das Configurações CNAB do Modo de Pagamento, agora o DIFF está menor e o PR está mais especifico, os commits também foram divididos, o que deve facilitar a Revisão. |
mbcosta
force-pushed
the
14.0-REF-unify_cnab_code_separate_cnab_from_payment_mode
branch
from
18b3e72
to
9efa25d
Compare
| tracking=True, | ||
| ) | ||
|
|
||
| comment = fields.Text() |
There was a problem hiding this comment.
Qual é a função desse campo? Notei que ele não está sendo utilizado. Isso realmente é necessário?
| <field name="name">CNAB Config</field> | ||
| <field name="res_model">l10n_br_cnab.config</field> | ||
| <field name="view_mode">tree,form</field> | ||
| <field name="context">{'group_by':['bank_id', 'payment_method_id']}</field> |
There was a problem hiding this comment.
Será que não seria melhor deixar o padrão sem agrupamento? O agrupamento automático prejudica a visualização e, o pior, não há como desabilitá-lo, ficando fixo. Seria mais interessante deixar a opção de agrupar disponível como filtro, mas não ativada por padrão.
|
pessoal, como o PR eh grande, ja vou dar o merge para nao aumentar o trabalho caso #3345 entrar antes. |
|
/ocabot merge nobump |
|
Hey, thanks for contributing! Proceeding to merge this for you. |
|
Congratulations, your PR was merged at b16d37e. Thanks a lot for contributing to OCA. ❤️ |
Unified CNAB Codes in only one object and separated CNAB Configuration from Payment Mode.
Unificando os Códigos CNAB em um único objeto e separando a Configuração do CNAB do Modo de Pagamento, apesar de alterar diversos arquivos nesse PR estão sendo feitas basicamento duas alterações:
1- Unificando os Códigos do CNAB
Para implementar o CNAB alguns campos foram deixados como CHAR e outros como objeto, inicialmente os "Códigos de Instrução e Retorno do Movimento" e recentemente foi incluído o "Código da Carteira" PR #2925 , olhando o PR sobre o "Código de Desconto" #3121 eu passei a considerar em fazer o mesmo que fiz para o "Código da Carteira" porque no Desconto
Os Bancos Bradesco e SICRED não enviam o caso "0 - Sem Desconto" o caso "1 - Valor Fixo" apenas varia de nomenclatura entre os Bancos porém hoje o código não permite os outros casos ex.: "2 - Percentual Até a Data Informada" , "3 - Valor por Antecipação Dia Corrido", etc e o Bradesco e SICRED parecem enviar o código "7 - Cancelamento de Desconto" ( é preciso confirmar se está faltando preencher esse campo para cancelar o desconto ).
OBS.: Nesse PR não está sendo resolvido essa questão do Desconto o PR já acabou sendo muito extenso e achei melhor deixar para outro PR para diminuir o "DIFF" e facilitar a revisão
Mas ao ver de transformar o campo em um objeto vi que essa solução estava gerando muito Código, Visões e Menus semelhantes quase duplicadas, e se isso fosse feito nos outros campos "Código de Tarifa, Abatimento e Multa" essa quantidade de código seria maior ainda por isso procurei uma forma de reduzir isso, o que fiz foi criar um objeto único "Código CNAB"/l10n_br_cnab.code e incluir um campo Seleção/selection "Tipo do Código"/code_type assim com um único objeto, visão e menu será possível incluir esses códigos evitando esse problema, coloquei a maioria dos possíveis casos
selection=[ ("instruction_move_code", "Instruction Move Code"), ("return_move_code", "Return Move Code"), ("wallet_code", "Wallet Code"), ("discount_code", "Discount Code"), ("protest_code", "Protest Code"), ("fee_code", "Fee Code"), ("interest_code", "Interest Code"), ("boleto_species", "Boleto Species"), ("boleto_accept", "Boleto Accept"), ("doc_finality_code", "DOC Finality Code"), ("ted_finality_code", "TED Finality Code"), ("complementary_finality_code", "Complementary Finality Code"), ("service_type", "Service Type"), ("release_form", "Release Form"), ("boleto_wallet", "Boleto Wallet"), ("boleto_modality", "Boleto Modality"), ("boleto_variation", "Boleto Variation"), ],Ao acessar o Faturamento > Configurações é possível acessar o CNAB Codes
Para tentar facilitar a revisão eu separei esse commit em dois o segundo tem apenas as alterações no arquivos data/cnab_codes que são o que acabam gerando mais volume no DIFF
2 - Separando as Configurações do CNAB do Modo de Pagamento
Fazendo a unificação dos códigos do CNAB acabei precisando, devido os scripts de migração, melhorar a definição de alguns campos por isso acabei decidindo fazer um melhoria que é essa separação do que são as informações do CNAB e do Modo de Pagamento/account.payment.mode para isso eu criei um novo objeto o l10n_br_cnab.config e agora no Modo de Pagamento é apenas informado o CNAB usado
Faturamento > Configurações > Modo de Pagamento
Faturamento > Configurações > CNAB Config
Isso é importante porque:
Sobre a Migração
Os scripts de migração devem criar todos os
Depois de rodar a migração você pode verificar esses objetos para confirmar se tudo ocorreu como deveria
IMPORTANTE: Não atualize um Banco de Dados em Produção diretamente crie um novo Banco para Homologação em caso de algum erro por favor veja de colocar aqui o LOG com a mensagem.
Inicialmente não foi possível excluir o que não será mais usado porque o script de migração precisa buscar os dados, algo semelhante de quando o openupgrade deixa alguns campos como "old", mas isso também é importante para em caso de algum problema os dados ainda estarão no BD mesmo que para uma eventual consulta o que acredito que também vai garantir uma migração tranquila, porém isso vai ser algo temporário e nos próximos PRs eu pretendo ver de eliminar esses objetos, campos e visões.
cc @rvalyi @renatonlima @marcelsavegnago @mileo