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

[12.0] Importação de NF-e por XML #1701

Closed
wants to merge 59 commits into from

Conversation

lfdivino
Copy link
Member

@lfdivino lfdivino commented Nov 8, 2021

Olá Pessoal,

Este PR visa viabilizar a importação de NF-e:

  • Importação do arquivo XML manualmente

Ele é uma separação da importação manual do PR #1355

@rvalyi @renatonlima @marcelsavegnago @mileo @gabrielcardoso21 Criticas e sugestões são sempre bem vindas

@rvalyi
Copy link
Member

rvalyi commented Nov 8, 2021

OK, como eu ja comentei eu acho importante ter esse sistema minimalista. Estrategias de depare ou de importação usando o DFe podem ser plugadas em cima disso em módulos opcionais, mas assim me parece no caminho certo, eu bolei esse dry_run exactamente para isso...

@marcelsavegnago
Copy link
Member

marcelsavegnago commented Nov 8, 2021

OK, como eu ja comentei eu acho importante ter esse sistema minimalista. Estrategias de depare ou de importação usando o DFe podem ser plugadas em cima disso em módulos opcionais, mas assim me parece no caminho certo, eu bolei esse dry_run exactamente para isso...

@rvalyi seria melhor a importação ficar em um modulo separado do l10n_br_nfe ? tipo l10n_br_nfe_import ou nao se faz necessário ?

@rvalyi
Copy link
Member

rvalyi commented Nov 8, 2021

OK, como eu ja comentei eu acho importante ter esse sistema minimalista. Estrategias de depare ou de importação usando o DFe podem ser plugadas em cima disso em módulos opcionais, mas assim me parece no caminho certo, eu bolei esse dry_run exactamente para isso...

@rvalyi seria melhor a importação ficar em um modulo separado do l10n_br_nfe ? tipo l10n_br_nfe_import ou nao se faz necessário ?

Eu acho melhor sim. Mas dá para ter um wizardzinho assim com 150 linhas, isso TB não cria nenhum monstro desse jeito. O foco mesmo deve ser em mapear os impostos na importação.

Copy link
Member

@renatonlima renatonlima left a comment

Choose a reason for hiding this comment

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

O wizard de importação deveria ser herdado do wizard: https://github.com/OCA/l10n-brazil/blob/12.0/l10n_br_fiscal/wizards/base_wizard_mixin.py#L8

A ideia é implementar a importação via arquivo (e futuramente via download) mas utilizando o mesmo wizard.

Captura de Tela 2021-11-08 às 15 17 39

Permitindo as opções:

Captura de Tela 2021-11-08 às 15 17 52

A ter uma pré-visutalização:

Captura de Tela 2021-11-08 às 15 23 11

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.

@lfdivino como eu disse nao vejo grande problema conceitual (so fazer o que o @renatonlima pediu). Mas tem que botar o Travis e Runbot verde tb e ver detalhadamente se nao veio algum lixo junto da branch que vc usou nesse PR.

@renatonlima sobre a pre-visualização tem que ver o que é facil razoavel fazer nesse modulo. Porque talvez isso ja é mais trabalho e tem que ser feito em outros modulos que vaõ herdar isso.

@lfdivino lfdivino force-pushed the feature/import_nfe_xml branch from 2906125 to 6b9dccd Compare November 17, 2021 13:34
@lfdivino lfdivino force-pushed the feature/import_nfe_xml branch from 2871b9d to e2a9d31 Compare November 30, 2021 16:27
@antoniospneto
Copy link
Contributor

antoniospneto commented Dec 1, 2021

@lfdivino tem que dar uma olhada que da forma que está o campo carrier_id está em conflito com o carrier_id que já existe definido no módulo l10n_br_delivery

Aqui na PR o carrier_id está relacionado a um partner_id mas no módulo l10n_br_delivery está relacionado a um delivery.carrier

Por causa disso acaba estourando esse erro:

psycopg2.IntegrityError: insert or update on table "l10n_br_fiscal_document" violates foreign key constraint "l10n_br_fiscal_document_carrier_id_fkey"
DETAIL:  Key (carrier_id)=(10) is not present in table "res_partner".

Obs: o teste foi feito em um banco de dados que já tem o módulo l10n_br_delivery com dados inseridos.

@lfdivino lfdivino force-pushed the feature/import_nfe_xml branch 9 times, most recently from 65f9a23 to ff17f9c Compare December 3, 2021 17:58
@mileo
Copy link
Member

mileo commented Dec 6, 2021

Pessoal ta muito estranho esse erro do travis, alguém tem alguma ideia?

@mileo mileo requested review from rvalyi and renatonlima December 6, 2021 12:59
@hirokibastos hirokibastos force-pushed the feature/import_nfe_xml branch from 2dcc959 to 085253a Compare May 10, 2022 19:11
@marcelsavegnago
Copy link
Member

@hirokibastos @lfdivino consegue rodar o pre-commit ? o interessante eh rodar sempre o pre-commit antes de comitar

@marcelsavegnago
Copy link
Member

@hirokibastos vc consegue dar uma olhada neste link e fazer o processo indicado por ele ? caso já tenha feito só da um toque aqui fazendo favor
https://odoo-community.org/page/cla

@rvalyi
Copy link
Member

rvalyi commented May 11, 2022

@lfdivino @hirokibastos @mileo eu tambem insisto na questão que fazer uns git squash e de quebrar essa PR em pedaços menores, tem coisas que são faceis de aprovar, basta extrair algumas mudanças simples primeiro. Senão vai ser impossivel fazer os cherry-picks pro port depois, eu ja tinha comentado sobre isso um mes atras #1701 (comment)

Vcs não podem esperar que a gente faça todo trabalho de arquitetura, de manutenção de migração no projeto e depois exigir que a gente faça merge de PRs de mais 1000 linhas ou 70 commits que não passou por nenhum crivo de qualidade e depois largar isso no repo e passar a outra coisa, como já foi feito tantas vezes... cc @marcelsavegnago @netosjb @renatonlima

@hirokibastos hirokibastos force-pushed the feature/import_nfe_xml branch from 8b4e6aa to b327ca8 Compare May 13, 2022 17:58
@marcelsavegnago
Copy link
Member

@hirokibastos vc consegue dar uma olhada neste link e fazer o processo indicado por ele ? caso já tenha feito só da um toque aqui fazendo favor https://odoo-community.org/page/cla

@hirokibastos você chegou a dar uma olhada neste link ?

@hirokibastos
Copy link
Contributor

@hirokibastos vc consegue dar uma olhada neste link e fazer o processo indicado por ele ? caso já tenha feito só da um toque aqui fazendo favor https://odoo-community.org/page/cla

@hirokibastos você chegou a dar uma olhada neste link ?

@marcelsavegnago Está feito, perdão pela demora com a resposta.

@lluisgustavoreis
Copy link

@lfdivino @hirokibastos @mileo eu tambem insisto na questão que fazer uns git squash e de quebrar essa PR em pedaços menores, tem coisas que são faceis de aprovar, basta extrair algumas mudanças simples primeiro. Senão vai ser impossivel fazer os cherry-picks pro port depois, eu ja tinha comentado sobre isso um mes atras #1701 (comment)

Vcs não podem esperar que a gente faça todo trabalho de arquitetura, de manutenção de migração no projeto e depois exigir que a gente faça merge de PRs de mais 1000 linhas ou 70 commits que não passou por nenhum crivo de qualidade e depois largar isso no repo e passar a outra coisa, como já foi feito tantas vezes... cc @marcelsavegnago @netosjb @renatonlima

Como você sugere que o PR seja dividido? Pois não vemos impacto da forma como está

@antoniospneto
Copy link
Contributor

@lluisgustavoreis

Um exemplo que eu vejo que já poderia ser extraído para um PR separada é a questão do transporte:
image
image
image

@rvalyi
Copy link
Member

rvalyi commented May 24, 2022

@lluisgustavoreis

Um exemplo que eu vejo que já poderia ser extraído para um PR separada é a questão do transporte:
image
image
image

É um bom exemplo sim @netosjb

@marcelsavegnago
Copy link
Member

marcelsavegnago commented May 27, 2022

@hirokibastos @lfdivino vocês acreditam que cabe alguns squashs para deixar o histórico mais limpo? Olhando por cima me parece que tem commtis corrigindo outros commits da mesma PR.

@rvalyi
Copy link
Member

rvalyi commented Jul 15, 2022

Vou fechar essa PR porque foi quebrado em PRs menores que já tiveram merge para a grandes maioria deles. Os pequenos que ficaram estão ainda visível para receber revisões e ajustes. Obrigado a todos que participaram!

@rvalyi rvalyi closed this Jul 15, 2022
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.