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.

Sign up for GitHub

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

Jump to bottom

[14.0][RFC] l10n_br_account_payment_brcobranca - fields Santander CNAB 400 #2870

Closed
wants to merge 2 commits into from
Closed

[14.0][RFC] l10n_br_account_payment_brcobranca - fields Santander CNAB 400 #2870

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
71967be
[RFC] l10n_br_account_payment_brcobranca: boleto_wallet2 rename to bo…
kaynnan Jan 16, 2024
9e1edd8
[RFC] l10n_br_account_payment_order: remove transmission_code and set…
kaynnan Jan 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion l10n_br_account_payment_brcobranca/models/account_move_line.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,9 @@ def send_payment(self):
if bank_account_id.bank_id.code_bc in ("033"):
boleto_cnab_api_data.update(
{
"carteira": str(move_line.payment_mode_id.boleto_wallet2),
"carteira": str(
move_line.payment_mode_id.boleto_wallet_to_impress
),
}
)

Expand Down
2 changes: 1 addition & 1 deletion l10n_br_account_payment_brcobranca/models/account_payment_order.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def _prepare_remessa_santander_400(self, remessa_values):
remessa_values.update(
{
"codigo_carteira": str(self.payment_mode_id.boleto_wallet),
"codigo_transmissao": self.payment_mode_id.transmission_code,
"codigo_transmissao": int(self.payment_mode_id.code_convetion),
"conta_corrente": misc.punctuation_rm(
self.journal_id.bank_account_id.acc_number
),
Expand Down
26 changes: 10 additions & 16 deletions l10n_br_account_payment_order/models/l10n_br_cnab_boleto_fields.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Copy link
Contributor

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.

Copy link
Contributor Author

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?

Copy link
Contributor

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

image

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.

Copy link
Contributor Author

@kaynnan kaynnan Jan 17, 2024 •
edited
Loading

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

tracking=True,
)

Expand Down Expand Up @@ -51,21 +51,6 @@ class L10nBrCNABBoletoFields(models.AbstractModel):
tracking=True,
)

boleto_wallet2 = fields.Char(
string="Carteira Boleto",
help="Código da carteira para ser impresso no boleto, "
"quando o mesmo for diferente do impresso na remessa.",
size=3,
tracking=True,
)

transmission_code = fields.Char(
string="Código de Transmissão",
help="Informação cedida pelo banco que identifica o arquivo remessa do cliente",
size=20,
tracking=True,
)

boleto_modality = fields.Char(
string="Modalidade",
size=2,
Expand Down Expand Up @@ -322,3 +307,12 @@ class L10nBrCNABBoletoFields(models.AbstractModel):
" usado pelos bancos Sicred/Unicred e Sicoob.",
tracking=True,
)

# Santander
boleto_wallet_to_impress = fields.Char(
string="Carteira Boleto",
help="Código da carteira para ser impresso no boleto, "
"quando o mesmo for diferente do impresso na remessa.",
size=3,
tracking=True,
)
6 changes: 4 additions & 2 deletions l10n_br_account_payment_order/views/account_payment_mode.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,12 @@
attrs="{'required': [('payment_method_code', 'in', ('240', '400', '500')), ('payment_type', '==', 'inbound')]}"
/>
<field name="boleto_wallet" />
<field name="boleto_wallet2" />
<field
name="boleto_wallet_to_impress"
attrs="{'invisible': [('bank_code_bc', 'not in', ('033') )]}"
Copy link
Contributor

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

Copy link
Contributor Author

@kaynnan kaynnan Jan 17, 2024 •
edited
Loading

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

Copy link
Contributor

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

Copy link
Contributor Author

@kaynnan kaynnan Jan 17, 2024 •
edited
Loading

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.

/>
<field name="boleto_modality" />
<field name="boleto_variation" />
<field name="transmission_code" />
<field name="boleto_accept" />
<field name="boleto_species" />
<field name="payment_method_code" invisible="1" />
Expand Down
Loading