l10n-brazil icon indicating copy to clipboard operation
l10n-brazil copied to clipboard

[14.0][l10n_br_fiscal][l10n_br_nfe][l10n_br_account][l10n_br_account_nfe] importação dos account.move com as NFe's - contra-proposta

Open rvalyi opened this issue 1 year ago • 38 comments

As announced, here is the Akretion alternative proposal for #2763, that is for importing account.move accounting and financial data along with NFe's or fiscal documents in general.

@mileo @hirokibastos as I warned you the code is totally different from what you proposed and as you can see I could not reuse even a single line of code. It took me 4 days and much more if considering I already spent lot's of time thinking about this ahead during these years when I designed the localization and specially the l10n_br_account module architecture and data-binding framework/xsdata/nfelib. So since you announced @hirokibastos would give it a try I was absolutely convinced it was out of reach from a beginner to come any close to such design. There was no way I could spend half an hour giving advice (unlike what I did for the dfe, cte, mdfe...) here and here and converging with something as complete and consistent as I'm proposing here:

key features:

  • [x] import NFe's along with their financial account.move (until now only l10n_br_fiscal.document was imported)
  • [x] import a set of several attachments right from the Vendor Bills native upload button (unlike #2763)
  • [x] import nearly all Brazilian tax values (l10n_br_nfe/models/document_line,py refactor de0d912da23c31d7b51226d72d6004b26cd0dd77 ). The tax computation is disabled so these values (amount, base, rate...) are kept as imported. This is done by extracting the bare minimum from #1412 (it's better to leave the rest up for debate in another PR, specially considering also https://github.com/OCA/l10n-brazil/pull/2559 ) and complemented with 1f07ec06d29b0719e04fc14e33ef9bfb5f3e85a0 This refactor to import all tax values is one of the reasons the diff is larger but IMHO we had to do it. The code is also a lot cleaner now and I could finally clean a useless/non orthogonal importation method introduced by Gabriel Cardoso on a hurry 3 years ago. We could merge that in a separate PR if that helps...
  • [x] accounting entries for Brazilian taxes are properly generated (not at all the case with #2763). See next point.
  • [x] account.move(.lines) are generated with odoo.test.common.Form so that all onchanges are called as expected just like if the account.move was filled manually. This avoids spaghetti code and glue modules that would otherwise be needed to play all the onchange of any combo of installed modules. This approach is already used in the OCA for years in modules such as OCA/rma. #2763 does not address this and open the door to lot's of logic duplication.
  • [x] NFe dups (payment terms) are properly imported but in a much smoother way than #2763 (avoid useless computations and risk of inconsistent data whenever anything would be recomputed in the account.move.). Proper payable/receivable account_id unlike #2763
  • [x] built atop of the composite.move refactor (that's a reason why the diff is a bit large but we could merge the composite.move PR first) so that a single account.move can now eventually have different l10n_br_fiscal.document from its invoice_line_ids. This can be useful to import several CTe's or several NFSe's under the same account.move as these fiscal document allow only 1 item line and as this is usually done in other ERP's. At least we are reusing the _onchange_fiscal_document_line_id from this feature to properly complete the account.move.line with their proper onchanges.
  • [x] most of the work is in l10n_br_account so that it will work with any fiscal document (Cte, MDFe...). On the contrary in the #2763 proposal most of the work was in l10n_br_account_nfe so it was poorly designed to work with other fiscal documents.
  • [x] rename some variables and methods from the previous importation wizard: xml can actually be a file of any format (XML and json are natively supported by xsdata and hence nfelib). Typically we "sniff" the uploaded file and only then determine the format so calling the file xml in 1st place was bad. Other bad names: xml while you were actually dealing with a "binding", calling a document what was actually a document_key... That the problem of letting a random sequence of beginners design a critic API... These renames are also one of the reasons the diff is larger but IMHO they are required.

cc @renatonlima @mbcosta @douglascstd

Now @mileo IMHO you could have anticipated better it would be a dead end to task @hirokibastos on this... Has he even 2 years of Odoo experience? He never got a PR merged yet in the OCA outside from the repo, right? Even KMEE got only one single module accepted outside from l10n-brazil in 10 years (to show the time in the POS...); meanwhile we got over 350... Even @marcelsavegnago got over 20 modules accepted in last 3 years only...

Instead, If you look the guys who design electronic invoicing in Europe, you will find Simone Orsi and Alexis de Lattre for OCA/edi, Alexis de Lattre for OCA/l10n-france, Pedro Baeza for OCA/l10n-spain... All guys with 20 years+ of professional experience and 15 years with Odoo, so there is no way would be any simpler in Brazil were we have the most complex electronic invoicing and tax engine in the world.... So again not @hirokibastos 's fault as I said. And as for "doing it my way", sorry you cannot tell something like that after KMEE did nearly no contribution since v12 during nearly 2 years waiting for us to do all the hard migration work. I also spent countless days helping you a ton for your CTe, MDFe and SPED requirements in the last 3 months... If you look at review or contribution statistics you can see I'm not really the one to blame: https://www.dixmit.com/en/blog/our-blog-1/ranking-of-top-contributors-to-the-odoo-community-association-oca-in-2022-8

So are you sure it's reasonable to point me as the culprit if KMEE beginner PRs are not always merged in OCA/l10n-brazil whenever you need some advanced feature?

Meanwhile, @marcelsavegnago @antoniospneto @felipemotter are living proofs that most serious contributions get very carefully reviews from me and make their way into the project.

rvalyi avatar Nov 09 '23 04:11 rvalyi

Hi @mbcosta, @antoniospneto, @felipemotter, @renatonlima, some modules you are maintaining are being modified, check this out!

OCA-git-bot avatar Nov 09 '23 06:11 OCA-git-bot

botei como "draft", para sinalizar que não estaria 100%. Falta uns detalhes muito pequeno e uns testes, Mas já funciona perfeitamente eu diria que esta 95%.

rvalyi avatar Nov 09 '23 11:11 rvalyi

@rvalyi , seria possível tratar a importação de nfe com com a tag autXML (vide #2765) neste commit?

A tag autXML traz apenas CNPJs, sem nomes para os contantos.

Acredito que a solução passar por

  1. tratar a extração da utXML para que se possa identificar o nome da pessoa jurídica e formatar os dados para que o contato possas seridentificado/criado

ou

  1. desprezar a informação das pessoas autorizadas, removendo o campo nfe40_autXML

rpsjr avatar Nov 09 '23 11:11 rpsjr

eu vi seu bug report, mas au ainda não resolvi. Vou olhar... Na pior vc acrescenta duas linhas para remover au autXML do binding antes de jogar no método import,_binding_nfe e aí contorna o problema por enquanto se vc tiver pressa. Mas com certeza vamos resolver.

rvalyi avatar Nov 09 '23 12:11 rvalyi

Legal @rvalyi, quero testar esta noite.

felipemotter avatar Nov 09 '23 13:11 felipemotter

Fala @rvalyi parabéns pelo trabalho absurdo. Está muito bom.

Fiz uma revisão básica aqui e alguns pontos que levantei:


Sobre a importação:

Como testei com os dados de um cliente modelo que temos, não posso compartilhar o xml que usei para testes, mas tentarei ser o mais claro possível:

Inconsistência 1: Ao tentar importar uma fatura com produtos que tenham unidades de medida(uom) de categorias diferentes (por exemplo: milheiro e quilo), acontece algum conflito bem grande, trocando produtos e até estourando erro em alguns casos. Eu não tive tempo suficiente para entender melhor.

Inconsistência 2: Importando uma nota apenas com uom de mesma categoria de uom, produtos que eram milheiro, por exemplo, acabaram ficando como unidade ao importar o move.

Vale ressaltar que no fiscal ta OK.


Sobre vários documentos fiscais para uma única fatura:

A abordagem parece legal, não testei a fundo, mas tenho uma dúvida: da forma que está colocado, só seria possível lançar uma fatura com vários documentos fiscais caso estes fossem importados, correto? Porque da forma que está hoje, só conseguimos criar um documento fiscal já vinculado a uma fatura.


Para deixar claro, realmente esta PR está muito mais madura e fluída do que a outra proposta.

felipemotter avatar Nov 10 '23 02:11 felipemotter

A abordagem parece legal, não testei a fundo, mas tenho uma dúvida: da forma que está colocado, só seria possível lançar uma fatura com vários documentos fiscais caso estes fossem importados, correto? Porque da forma que está hoje, só conseguimos criar um documento fiscal já vinculado a uma fatura.

Então meu objetivo aqui era evitar sabotar o projeto com a outra proposta... Acabei não me dedicando tanto na questão do move composto. Mas a gente poderia imaginar várias soluções. Por examplo essa PR já importa uma serie de documentos fiscais. Poderia ter um checkbox no wizard para escolher de juntar no mesmo account move se for possível. Aí ele apenas passaria o account.move criado no primeiro documento para importar os outros documentos. O método import_fiscal_document tem um parâmetro para isso. Poderia tambem escolher um account.move no wizard. Ou então a gente poderia imaginar um botão lá no account.move para abrir o wizard. Poderia tambem ter um Wizard que deixaria escolher um l10n_br_fiscal.document sem ser para importar ele. Aí talvez valeria a pena trazer de volta o menu que permite listar e editar os l10n_br_fiscal.document sem ser pela intermediação do account.move, assim como temos quando vc instala apenas o módulo l10n_br_fiscal antes de instalar o módulo l10n_br_account.

Vou olhar os outros pontos que vc levantou. Vc pode mandar sua nota pelo Telegram ou anonimizar ela e mandar aqui (EDIT: eu saquei o problema da unidade, já já arrumo).

rvalyi avatar Nov 10 '23 09:11 rvalyi

@felipemotter tinha algo zoado no wizard de depare da Kmee: logo que importava um produto uma vez zoava a unidade pelo jeito. Eu fiz um workaround no último commit. Agora a uom funciona com sua NFe por examplo.

Eu TB melhorei algumas outras coisas.

@rpsjr eu alterei para não importar mais o autXML, não deve mais criar problema, se puder confirmar...

Bem, ainda não tá perfeito, mas eu acho que não tá longe. Contribuições bem vindas.

rvalyi avatar Nov 12 '23 13:11 rvalyi

Falha no runboat >>

2023-11-13 00:33:37,232 329 CRITICAL bde23c8e1-a38d-42b6-93c9-7b2fdf651cb5 odoo.service.server: Failed to initialize database bde23c8e1-a38d-42b6-93c9-7b2fdf651cb5. Traceback (most recent call last): File "/opt/odoo/odoo/service/server.py", line 1201, in preload_registries registry = Registry.new(dbname, update_module=update_module) File "/opt/odoo/odoo/modules/registry.py", line 89, in new odoo.modules.load_modules(registry._db, force_demo, status, update_module) File "/opt/odoo/odoo/modules/loading.py", line 459, in load_modules processed_modules += load_marked_modules(cr, graph, File "/opt/odoo/odoo/modules/loading.py", line 347, in load_marked_modules loaded, processed = load_module_graph( File "/opt/odoo/odoo/modules/loading.py", line 249, in load_module_graph cr.commit() File "", line 2, in commit File "/opt/odoo/odoo/sql_db.py", line 101, in check return f(self, *args, **kwargs) File "/opt/odoo/odoo/sql_db.py", line 451, in commit flush_env(self) File "/opt/odoo/odoo/sql_db.py", line 78, in flush_env env_to_flush['base'].flush() File "/opt/odoo/odoo/models.py", line 5486, in flush self.recompute() File "/opt/odoo/odoo/models.py", line 5945, in recompute process(field) File "/opt/odoo/odoo/models.py", line 5929, in process field.recompute(recs) File "/opt/odoo/odoo/fields.py", line 1176, in recompute self.compute_value(recs) File "/opt/odoo/odoo/fields.py", line 1198, in compute_value records._compute_field_value(self) File "/opt/odoo/odoo/models.py", line 4082, in _compute_field_value odoo.fields.determine(field.compute, self) File "/opt/odoo/odoo/fields.py", line 82, in determine return needle(*args) File "/mnt/data/odoo-addons-dir/l10n_br_purchase/models/purchase_order_line.py", line 98, in _compute_amount line._update_taxes() File "/mnt/data/odoo-addons-dir/l10n_br_fiscal/models/document_fiscal_line_mixin_methods.py", line 291, in _update_taxes if self.document_id.imported_document: AttributeError: 'purchase.order.line' object has no attribute 'document_id'

rpsjr avatar Nov 13 '23 00:11 rpsjr

@rpsjr foi corrigido.

Pessoal, o problema principal que sobra é que tem algumas NFe's que o total não bate. Parece ser algo relacionado a soma de frete ou IPI no amount_total do account.move. Quando importa a NFe sem ter o modulo l10n_br_account instalado importa os totais certinho, mas com o l10n_br_account os totais não batem mais. Ai ele ate não importa as duplicatas nesse caso. Se alguém conseguir ajudar a resolver...

cc @renatonlima @marcelsavegnago @antoniospneto @felipemotter @mbcosta @hirokibastos

rvalyi avatar Nov 13 '23 15:11 rvalyi

mais informações sobre a questão dos totais:

  1. numa importação de NFe apenas com o modulo l10n_br_nfe instalado o amount_total da NFe bate porque temos nfe40_vNFe = related("amount_total") ai ao importar escreve o valor certo no amount_total sem recalcular o total, Mas logo que recalcula o total fica zoado se a nota tiver IPI ou frete por examplo.

  2. o ponto é que o amount_total das linhas não é correto. Pois normalmente dentro do metodo _compute_amounts do l10n_br_fiscal/models/document_fiscal_line_mixin_methods.py temos:

    def _compute_amounts(self):
        [....]
            record.amount_tax = record.amount_tax_not_included

            add_to_amount = sum([record[a] for a in record._add_fields_to_amount()])
            rm_to_amount = sum([record[r] for r in record._rm_fields_to_amount()])

            # Valor do documento (NF)
            record.amount_total = (
                record.amount_untaxed + record.amount_tax + add_to_amount - rm_to_amount
            )

Mas este calculo do amount_total não esta considerando os valores amount_tax, add_to_amount e rm_to_amount. Parece que estes ultimos campos estão zerados apesar da gente importar IPI ou frete corretamente por examplo. Então eu acho que o caminho é corrigir o codigo para que esses valores sejam calculados ou importados corretamente para depois a soma do amount_total ficar OK. A gente poderia até pensar em forçar o valors do vNF no amount_total do account.move. Mas ai ele não iria bater com o valor das linhas/lançamerntos e ai ele nem iria salvar.

rvalyi avatar Nov 13 '23 20:11 rvalyi

Realmente @rvalyi a minha preocupação é semelhante a questão que o magno levantou aquela vez, para forçar valores igual ao fiscal, porém o contábil continuar errado(princiál impacto seria o financeiro).

felipemotter avatar Nov 13 '23 23:11 felipemotter

Naquela época, eu até pensei em fazer algumas validações para que o valor total batesse com o fiscal, porém acabei não tendo tempo para tal. Isso forçaria a corrigir os problemas e com o tempo ficaria liso (o risco seria travar o faturamento de um usuário que estivesse com urgência)

felipemotter avatar Nov 13 '23 23:11 felipemotter

@rpsjr foi corrigido.

Não consegui validar a correção nesta instância: http://oca-l10n-brazil-14-0-pr2781-755c47cf259a.runboat.odoo-community.org/

Quero dizer, o erro persiste.

rpsjr avatar Nov 14 '23 00:11 rpsjr

@felipemotter so para registrar, eu matei a charada. pqp viu, esse tava digno daquele bug que a gente ficou cassando no inicio da v14... Assim que eu tiver tempo eu limpo um commit para subir...

rvalyi avatar Nov 14 '23 07:11 rvalyi

ai @felipemotter da uma testada com a sua NFe: é perfeito ou não é?

Screenshot from 2023-11-14 12-27-14 Screenshot from 2023-11-14 12-27-42

Screenshot from 2023-11-14 12-33-14

cc @antoniospneto @marcelsavegnago @renatonlima @mbcosta @hirokibastos @rpsjr

rvalyi avatar Nov 14 '23 15:11 rvalyi

@felipemotter eu tive esses prints localmente com o modulo l10n_br_account_nfe instalado. Porem ao testar com a mesma nota no Runboat (depois eu detonei) eu tive problema com as unidades e valores das duas primeiras linhas da NFe, como se tivesse rolado alguma conversão de unidade. Eu ainda não sei qual a razão disso no Runboat, mas devemos estar perto pois aqui apenas com l10n_br_account_nfe a note foi um espelho perfeito eu acho.

EDIT: algo que eu vi é que no Runboat as linhas account.move.line vem numa ordem diferente do meu localhost. Tem lugar no PR onde eu preciso da ordem das linhas. Talvez eu preciso ordenar por id ou ver esse ponto da ordenação melhor...

rvalyi avatar Nov 14 '23 16:11 rvalyi

@rvalyi vou ver se consigo dar uma olhada hoje

felipemotter avatar Nov 14 '23 17:11 felipemotter

@rvalyi vou ver se consigo dar uma olhada hoje

tava faltando esse patch ai que eu tinha feito no meu src/addons/account localmente e que agora eu botei no PR: https://github.com/OCA/l10n-brazil/pull/2781/commits/0d93c1c37b1aef7725b1e7aa4b17b5b6454d494a

Ai agora ta funcionando redondinho, até no Runboat com todos os modulos. Deve ter que acertar alguns detalhes para NFe's de importação ou importação mas as NFe's mais comum com ICMS, PIP, PIS, COFINS et até Difal me parece que funciona bem.

cc @antoniospneto @marcelsavegnago @renatonlima @mbcosta @hirokibastos @rpsjr

rvalyi avatar Nov 14 '23 21:11 rvalyi

Acabei de fazer o teste tanto localmente quanto pelo runboat usando vários xml, os erros que havia reportado sumiram. Realmente uma proposta bem madura e fluída. Será muito bem vinda.

felipemotter avatar Nov 15 '23 02:11 felipemotter

Vale a pena dizer que a gente implementou a importação das notas de forma semelhante na França faz uns 2 anos.

Me serveu TB muito a nossa experiência com os conectores e-commerce, pois é disso que eu tirei a ideia do odoo.test.common.Form para disparar os onchanges.

Nas próximas versões do Odoo a gente poderia pensar em ter mais campos calculados e menos onchanges na parte fiscal e na integração com o account. Porem o módulo account na v14 está ainda totalmente movido a onchange e por isso não teria tanto ganho possível e temos sim que lidar com os onchanges do módulo account mas TB dos modulos adicionais como os do módulo delivery por examplo. Enfim justificando aqui o design da solução...

rvalyi avatar Nov 15 '23 03:11 rvalyi

@renatonlima @marcelsavegnago @antoniospneto @felipemotter @mbcosta @hirokibastos @rpsjr

Eu dei um rebase e dei uma arrumada nos commits. Em especial:

  1. esse para popular os campos dos impostos: https://github.com/OCA/l10n-brazil/pull/2781/commits/e68963fea414e349987da278dca62759341c55c6
  2. e esse onde generalizei o calculo das taxas incluidas e excluidas a partir dos valores importados: https://github.com/OCA/l10n-brazil/pull/2781/commits/42c67ffa00b3541cb7aadd47470ba125fcc685bf

Agora me parece que esta funcionando muito bem... Alguns prints onde temos o match perfeito entre IPI NT, Isento, 0% e onde tem ICMS com redução de base...

2023-11-22_03-20 2023-11-22_03-16 2023-11-22_03-13

Seria interessante se as pessoas puderem testar de forma extensiva.

Depois, para fatiar o boi, eu posso extrair 2 PRs menores:

  1. com o account.move compostos
  2. um outro com esse commit e68963fea414e349987da278dca62759341c55c6

rvalyi avatar Nov 22 '23 06:11 rvalyi

@rvalyi você tem algum pensamento de como seria feita a vinculação com os purchase.order.line e os stock.move?

felipemotter avatar Nov 22 '23 22:11 felipemotter

@rvalyi você tem algum pensamento de como seria feita a vinculação com os purchase.order.line e os stock.move?

pro link com stock.picking, poderia ser um pequeno override no proprio l10n_br_stock_account e ai se reconhecer uma linha num picking de entrada, botaria a quantidade "done" da NFe. Se não reconhecer a linha poderia gerir a linha (ou ate o picking todo). Poderia passar a informação se algum picking ou PO deu match no wizard de importação. Depois caso houver diferença entre planejado e "done" ai eu diria que o operador ajustaria o backorder. Algo semelhante poderia acontecer no modulo l10n_br_purchase. cc @renatonlima

rvalyi avatar Nov 23 '23 02:11 rvalyi

Num cenário ideal, e considerando as empresas que necessitam fazer manifestação do destinatário, a NFe chega antes do recebimento, correto? Então o picking fica dependente do recebimento/importação da NFe de forma prévia. O picking estaria limitado às quantidades da NFe mas o picking mesmo é, em regra, posterior ao recebimento/importação da NFe.

Isto em mente, vislumbro um cenário em que a NFe importada ficaria latente para futuro e eventual picking. Uma vez qur o picking seja chamado dentro do pedido, da-se opção (pré preenchida) com os dados da NFe previamente importada.

On Wed, Nov 22, 2023, 23:26 Raphaël Valyi @.***> wrote:

@rvalyi https://github.com/rvalyi você tem algum pensamento de como seria feita a vinculação com os purchase.order.line e os stock.move?

pro link com stock.picking, poderia ser um pequeno override no proprio l10n_br_stock_account e ai se reconhecer uma linha num picking de entrada, botaria a quantidade "done" da NFe. Se não reconhecer a linha poderia gerir a linha (ou ate o picking todo). Poderia passar a informação se algum picking ou PO deu match no wizard de importação. Depois caso houver diferença entre planejado e "done" ai eu diria que o operador ajustaria o backorder. Algo semelhante poderia acontecer no modulo l10n_br_purchase. cc @renatonlima https://github.com/renatonlima

— Reply to this email directly, view it on GitHub https://github.com/OCA/l10n-brazil/pull/2781#issuecomment-1823750262, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKCBYEIY67KAULU5G2AD2D3YF2X53AVCNFSM6AAAAAA7D6B7SGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMRTG42TAMRWGI . You are receiving this because you were mentioned.Message ID: @.***>

rpsjr avatar Nov 23 '23 09:11 rpsjr

@rvalyi esse PR substitui o #2735 completamente? O que falta pra sair do draft? Tem algo que possamos ajudar?

DiegoParadeda avatar Feb 28 '24 15:02 DiegoParadeda

@rvalyi esse PR substitui o #2735 completamente? O que falta pra sair do draft? Tem algo que possamos ajudar?

eu diria que substitui completamente sim, pois como expliquei nao teve nenhum código que eu pude re aproveitar e enfim expliquei em detalhe porque.

Eu diria que a primeira coisa que vcs poderiam fazer é testar e ver se estão OK com o PR em geral ou se vcs enxergam alguns problemas.

Consigo dividir o PR em pedaços menores para ele ir entrando. Eh que a gente tinha até mais prioridade em migrar alguns módulos importantes para a v16...

rvalyi avatar Feb 28 '24 18:02 rvalyi

Até o momento, não tive nenhum erro com a importação de XML, somente ao importar ele não consegue identificar a Natureza da Operação, tendo de ser necessária a alteração manualmente após a importação. Sugestão, incluir o campo CFOP no Imported Document. Quanto a importação via DFE, mesmo colocando o Ultimo NSU, não funciona ele muda para 0 quando clico em Search Documents.

rodmad85 avatar Feb 29 '24 18:02 rodmad85

seria interessante revisar e fazer o merge do #2927 porque é um código que iria conflitar com esse PR. Acontece que o #2927 é tb um PR simples e que seria bom fazer antes de migrar o módulo para nao ter scripts de migração em varias versões...

rvalyi avatar Mar 04 '24 12:03 rvalyi

pessoal, eu dei um rebase neste PR.

  • E também eu refiz esse "sub-PR" https://github.com/OCA/l10n-brazil/pull/2761 que seria bom mesclar primeiro para ir simplificando. Ele não ta 100% mas da para começar a avaliar...
  • E acabei de extrair esse novo que eu acho que ta tranquilo: https://github.com/OCA/l10n-brazil/pull/2939
  • E eu extrai esse tb: #2940

cc @rodmad85 @renatonlima @mbcosta @antoniorubiopesol @felipemotter @marcelsavegnago @DiegoParadeda

rvalyi avatar Mar 04 '24 21:03 rvalyi