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][IMP] l10n_br_nfe: Emissão da NFCe #2329

Merged
merged 18 commits into from
Aug 10, 2023
Merged

[14.0][IMP] l10n_br_nfe: Emissão da NFCe #2329

merged 18 commits into from
Aug 10, 2023

Conversation

ygcarvalh
Copy link
Contributor

@ygcarvalh ygcarvalh commented Feb 2, 2023 •
edited by mileo
Loading

Bom, fiz algumas alterações no módulo l10n_br_nfe visando a emissão da NFCe, mas sem a necessidade de alterar os fluxos das coisas que já funcionam atualmente. A remoção de algumas tags (como IPI e a data e hora de entrada/saída) vão ser alteradas apenas quando o tipo de documento selecionado for o 65. Outro ponto foi em relação ao fluxo, como a NFCe pode ter ambos os casos (síncrono e assíncrono), foi necessário realizar uma validação no retorno para lidar com esse caso.

Houve também uma alteração realizada na biblioteca do erpbrasil.edoc que adiciona uma nova classe da NFCe, assim como uma validação para criarmos esse objeto quando o tipo de documento selecionado for esse.

Além disso, uma nova configuração foi adicionada a empresa. A NFCe precisa, obrigatoriamente, do CSC Token e do Código emitido pelo próprio SEFAZ, além da versão do QR-Code (1 ou 2).

Próximos passos:

  • Geração de testes para NFCe

Depende das alterações que foram realizadas no PR erpbrasil/erpbrasil.edoc#51

Sugestões, dúvidas e críticas são sempre bem-vindas.

@OCA-git-bot
Copy link
Contributor

Hi @rvalyi, @renatonlima,
some modules you are maintaining are being modified, check this out!

@mileo mileo changed the title [14.0][IMP] l10n_br_nfce: Emissão da NFCe [14.0][IMP] l10n_br_nfe: Emissão da NFCe Feb 2, 2023
@ygcarvalh ygcarvalh changed the title [14.0][IMP] l10n_br_nfe: Emissão da NFCe [14.0][WIP] l10n_br_nfe: Emissão da NFCe Feb 23, 2023
@ygcarvalh ygcarvalh force-pushed the feature/nfce branch 3 times, most recently from 4629885 to 26d7a74 Compare March 13, 2023 17:45
@ygcarvalh ygcarvalh force-pushed the feature/nfce branch 2 times, most recently from ee64f7a to 4c7ab4e Compare March 22, 2023 14:49
@ygcarvalh
Copy link
Contributor Author

Adicionado dependência das alterações do PR #2395.

@lfdivino
Copy link
Member

LGTM

@ygcarvalh ygcarvalh changed the title [14.0][WIP] l10n_br_nfe: Emissão da NFCe [14.0][IMP] l10n_br_nfe: Emissão da NFCe May 24, 2023
@ygcarvalh ygcarvalh marked this pull request as ready for review May 24, 2023 18:46
@marcelsavegnago
Copy link
Member

marcelsavegnago commented May 24, 2023 •
edited
Loading

@lfdivino @felipezago.. está com erro nos testes e no runboat.. Está aprovado mesmo ?

@marcelsavegnago
Copy link
Member

parece que tem um erro no runboat mas acredito que a origem pode ser de outro lugar.. vi o mesmo erro em outra PR se nao me engano

@ygcarvalh
Copy link
Contributor Author

@marcelsavegnago o erro nos testes eu estou investigando o que pode ter sido, o do runbot tinha dado em outro PR meu e localmente tentando subir com dado de demo...

@marcelsavegnago
Copy link
Member

@marcelsavegnago o erro nos testes eu estou investigando o que pode ter sido, o do runbot tinha dado em outro PR meu e localmente tentando subir com dado de demo...

Eh então eu vi que aconteceu em outras PRs .. vlwww

@ygcarvalh ygcarvalh force-pushed the feature/nfce branch 2 times, most recently from 8d1132e to 66e4de3 Compare May 26, 2023 11:16
mileo
mileo approved these changes May 26, 2023
View reviewed changes
Copy link
Member

@mileo mileo left a comment

Choose a reason for hiding this comment

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

LGTM

mileo
mileo approved these changes Aug 8, 2023
View reviewed changes
Copy link
Member

@mileo mileo left a comment

Choose a reason for hiding this comment

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

LGTM

antoniospneto
antoniospneto requested changes Aug 9, 2023
View reviewed changes
l10n_br_nfe/models/document.py Outdated

if infProt.cStat in AUTORIZADO:
try:
record.make_pdf()
Copy link
Contributor

Choose a reason for hiding this comment

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

Não seria melhor criar o PDF em outro momento ou em segundo plano ?
Mesmo fazendo o tratamento da exceção para não interromper o fluxo, essa é uma operação muito critica.
Se por algum motivo a requisição demorar muito, o odoo vai "matar" essa requisição por esperar muito tempo.
Então penso que quanto menos processamento a gente fizer nessa hora melhor, querendo ou não essa geração do PDF é bem pesada.

Copy link
Member

Choose a reason for hiding this comment

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

Também não vejo motivo para gerarmos o PDF durante a transmissão.

rvalyi
rvalyi approved these changes Aug 10, 2023
View reviewed changes
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.

LGTM, parabéns pelo trabalho!

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

lfdivino
lfdivino approved these changes Aug 10, 2023
View reviewed changes
Copy link
Member

@lfdivino lfdivino left a comment

Choose a reason for hiding this comment

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

LGTM!

@rvalyi
Copy link
Member

rvalyi commented Aug 10, 2023

/ocabot merge major

@OCA-git-bot
Copy link
Contributor

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

@OCA-git-bot OCA-git-bot merged commit 34b2b23 into OCA:14.0 Aug 10, 2023
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 9c0eb61. Thanks a lot for contributing to OCA. ❤️

@antoniospneto antoniospneto mentioned this pull request Aug 11, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mileo mileo mileo approved these changes

@felipezago felipezago felipezago approved these changes

@lfdivino lfdivino lfdivino approved these changes

@antoniospneto antoniospneto antoniospneto approved these changes

@rvalyi rvalyi rvalyi approved these changes

@renatonlima renatonlima Awaiting requested review from renatonlima

@marcelsavegnago marcelsavegnago Awaiting requested review from marcelsavegnago

Assignees
No one assigned
Labels
14.0 approved merged 🎉 new feature ready to merge work in progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@ygcarvalh @OCA-git-bot @lfdivino @marcelsavegnago @mileo @felipezago @rvalyi @antoniospneto @kmee