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

[14.0][REF][l10n_br_fiscal][l10n_br_fiscal_edi] l10n_br_fiscal_edi extraction from l10n_br_fiscal #3012

Merged
merged 7 commits into from
Sep 7, 2024

Conversation

rvalyi
Copy link
Member

@rvalyi rvalyi commented Apr 12, 2024

Este PR extrai um modulo l10n_br_fiscal_edi do modulo l10n_br_fiscal. Pois o módulo l10n_br_fiscal que esta usado por muitos módulos tem que focar em ser apenas o tax engine da localização e coisas como importação de documentos fiscais, eventos de comunicação fiscal ou hooks de mudança de estado do documento fiscal devem ser extraídos neste modulo extra l10n_br_fiscal_edi que hoje apenas 3 modulos iriam usar: l10n_br_nfe, l10n_br_nfse e l10n_br_fiscal_closing.

Eh previsível também que logo mais coisas serão adicionadas no modulo l10n_br_fiscal_edi e por isso é urgente começar a dividir quanto antes. Futuramente da v16 para frente, algumas coisas poderão ser inspiradas dos trabalhos feitos em https://github.com/OCA/edi/ e https://github.com/OCA/edi-framework porem o primeiro passo já eh começar por extrair o módulo...

l10n_br_fiscal antes:

-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
CSV                             24              0              0          32260
XML                             83           1155            485          16645
Python                         110           2607            826          13074
PO File                          1           1668           6315           3371
HTML                             1             86              6            458
reStructuredText                 6            123             48            187
-------------------------------------------------------------------------------
SUM:                           225           5639           7680          65995
-------------------------------------------------------------------------------

l10n_br_fiscal depois:

-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
CSV                             24              0              0          32255
XML                             75           1128            471          16174
Python                         103           2378            749          11959
PO File                          1           1668           6315           3371
HTML                             1             86              6            458
reStructuredText                 6            123             48            187
-------------------------------------------------------------------------------
SUM:                           210           5383           7589          64404
-------------------------------------------------------------------------------

ou seja tira 1591 linhas do modulo l10n_fiscal e impacta (levemente) quase que apenas 3 modulos l10n_br_nfe, l10n_br_nfse e l10n_br_fiscal_closing que passam então a depender deste novo modulo l10n_br_fiscal_edi.

Combinado com o #3290 que tira umas 500 outras linhas, já traz do modulo l10n_br_fiscal dentro de um tamanho mais razoável de umas 9500 linhas de Python (quando tira os testes).

Para nos da Akretion é indispensável fazer essa extração na v16 porque não da pro módulo l10n_br_fiscal ter um scope infinito e passar de 12k linhas de Python ou de 20k linhas qdo considera o XML (que da o mesmo tanto de trabalho para maintainer). A única questão é se a gente faz na v14 tb e mantém uma boa sinergia pros cherry-picks entre as branches 14 e 16 para essas 1600 linhas do l10n_br_fiscal_edi ou se a gente fica conservador na v14 mas com certeza terá menos backport de debug da v14 daqui 1 ou 2 anos. Eu diria que vale a pena fazer a mudança na v14, mas cabe a os que ajudaram na v14 de decidir o que querem para vcs @marcelsavegnago @antoniospneto @felipemotter @renatonlima @mileo

@rvalyi rvalyi marked this pull request as draft April 12, 2024 02:42
@rvalyi rvalyi force-pushed the 14.0-fiscal-edi-add branch 3 times, most recently from a342b96 to bc9c6f2 Compare April 12, 2024 04:12
@rvalyi rvalyi changed the title [14.0][REF][POC][l10n_br_fiscal][l10n_br_fiscal_edi] l10n_br_fiscal_edi extraction from l10n_br_fiscal 2/2 [14.0][REF]l10n_br_fiscal][l10n_br_fiscal_edi] l10n_br_fiscal_edi extraction from l10n_br_fiscal 2/2 Apr 12, 2024
@rvalyi rvalyi changed the title [14.0][REF]l10n_br_fiscal][l10n_br_fiscal_edi] l10n_br_fiscal_edi extraction from l10n_br_fiscal 2/2 [14.0][REF][l10n_br_fiscal][l10n_br_fiscal_edi] l10n_br_fiscal_edi extraction from l10n_br_fiscal 2/2 Apr 12, 2024
@OCA-git-bot
Copy link
Contributor

Hi @luismalta, @mileo, @gabrielcardoso21, @renatonlima, @mbcosta, @marcelsavegnago,
some modules you are maintaining are being modified, check this out!

@rvalyi rvalyi force-pushed the 14.0-fiscal-edi-add branch 3 times, most recently from 8de978b to 30c46d7 Compare May 24, 2024 21:35
@rvalyi
Copy link
Member Author

rvalyi commented May 24, 2024

Pessoal, dei um rebase porque o PR do @felipemotter #3090 matou a metade dos workarounds dessa PR (ver detalhe dos commits). Nisso a extração do l10n_br_fiscal_edi ta bem facil agora...

@renatonlima
Copy link
Member

Olá pessoal,

Há algum tempo atrás eu havia trabalhado para melhorar o suporte aos documentos fiscais principalmente os eletrônicos para implementar algumas funcionalidades faltantes como o MDE, DFE e demais eventos dos documentos fiscais, para isso é necessário primeiro fazer uma organização na estrutura base dos documentos fiscais, porque os objetos e heranças dos documentos fiscais devem, ser simplificado, eu fiz um diagrama atualmente dos objetos dos documentos fiscais:

classDiagram
    FiscalDocumentEletronic <|-- FiscalDocumentWorkflow
    FiscalDocument <|-- FiscalDocumentEletronic
    FiscalDocumentMixin <|-- FiscalDocumentMixinFields
    FiscalDocumentMoveMixin <|-- FiscalDocumentMixinMethods
    FiscalDocumentMixin <|-- FiscalDocumentMixinMethods
    FiscalDocument <|-- FiscalDocumentMoveMixin
    FiscalDocument <|-- FiscalDocumentMixinFields
    FiscalDocument <|-- MailThread
    class FiscalDocumentWorkflow{
        state_edoc = fields.Selection
        state_fiscal = fields.Selection
        cancel_reason = fields.Char
        correction_reason = fields.Char()
        +action_document_confirm()
        +action_document_send()
        +document_back2draft()
        +action_document_back2draft()
        +action_document_cancel()
        +action_document_invalidate()
        +action_document_correction()
    }
    class FiscalDocumentEletronic {
        issuer = fields.Selection
        status_code = fields.Char
        status_name = fields.Char
        status_description = fields.Char  
        authorization_event_id = fields.Many2one
        authorization_date = fields.Datetime
        authorization_protocol = fields.Char
        send_file_id = fields.Many2one
        authorization_file_id = fields.Many2one
        cancel_event_id = fields.Many2one
        cancel_date = fields.Datetime
        cancel_protocol_number = fields.Char
        cancel_file_id = fields.Many2one
        invalidate_event_id = fields.Many2one
        invalidate_date = fields.Datetime
        invalidate_protocol_number = fields.Char
        invalidate_file_id = fields.Many2one
        document_version = fields.Char
        is_edoc_printed = fields.Boolean
        file_report_id = fields.Many2one
        +serialize()
        +view_xml()
        +make_pdf()
        +view_pdf()

    }
Loading

Alguns objetos e heranças podem ser simplificados eu comecei a fazer esse trabalho aqui na branch 14.0-rfc-fiscal-document-events onde eu também fiz melhorias na gestão da comunicação com a Sefaz e suporte a outros webservice ainda não implementados no Odoo. Eu vou fazer um rebase e continuar esse trabalho.

cc @rvalyi @antoniospneto @marcelsavegnago @mileo @mbcosta

@rvalyi rvalyi force-pushed the 14.0-fiscal-edi-add branch 2 times, most recently from 450aef0 to a14ced1 Compare September 1, 2024 22:15
@rvalyi
Copy link
Member Author

rvalyi commented Sep 1, 2024

@antoniospneto fiz o merge do seu PR e o merge no primeiro commit do PR assim que eu expliquei. Eu tb testei num banco de prod e deu tudo certo.

@marcelsavegnago algum posicionamento sobre o PR?

Copy link
Contributor

@antoniospneto antoniospneto left a comment

Choose a reason for hiding this comment

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

Testei em uma base cópia de produção, com as ultimas melhorias mescladas aqui e nenhum erro foi encontrado.
Os módulos foram atualizados com sucesso.

Testei as operações de NF-e em homologação, transmissão de nfe, cancelamaneto, carta de correção e inutilização de número, todas ok.

Obs: não abordei nos meus testes: NFC-e e NFS-e.

Quanto a revisão do código, pelo que vi tá tudo ok, o readme ficou bem claro.
Alguma coisa pode ter passado despercebido, pois é bastante alteração, mas nada que não possa ser corrigido em seguida..

Acho que o importante é todos estarem cientes deste refactor, para que quando forem atualizar a base, atualizar com atenção e homologar os processos antes de por em produção.

Então por mim já pode rolar o merge, valeu @rvalyi, mais uma super contribuição para deixar o localização mais robusta.

@marcelsavegnago
Copy link
Member

Bora... Quanto mais cedo enfrentar esta questão.. melhor.

@rvalyi
Copy link
Member Author

rvalyi commented Sep 2, 2024

@renatonlima teve tempo para olhar?

@rvalyi
Copy link
Member Author

rvalyi commented Sep 7, 2024

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

@rvalyi The merge process could not start, because command `git merge --no-ff -m 'Merge PR #3012 into 14.0

Signed-off-by rvalyi' tmp-pr-3012` failed with output:

Auto-merging README.md
Auto-merging l10n_br_account/i18n/l10n_br_account.pot
Auto-merging l10n_br_account/i18n/pt_BR.po
Auto-merging l10n_br_account/models/account_move.py
Auto-merging l10n_br_account/models/fiscal_document.py
Auto-merging l10n_br_fiscal/__manifest__.py
CONFLICT (content): Merge conflict in l10n_br_fiscal/__manifest__.py
Auto-merging l10n_br_fiscal/static/description/index.html
CONFLICT (content): Merge conflict in l10n_br_fiscal/static/description/index.html
Auto-merging l10n_br_fiscal/views/document_view.xml
Auto-merging l10n_br_nfe/__manifest__.py
Automatic merge failed; fix conflicts and then commit the result.

[REF] extract l10n_br_fiscal_edi from l10n_br_fiscal 2/2

[TMP] leave invalidate.number in l10n_br_fiscal

[REF] l10n_br_fiscal: mv doc wrkflw ->fisc edi

[REF] l10n_br_fiscal_edi: mv doc wrkflw ->fisc edi

[WIP] mv edoc import to fiscal_edi

[REF] add fiscal document hook

[FIX] l10n_br_fiscal_edi:  wrong status_description

[FIX] l10n_br_fiscal: edi migration
@rvalyi
Copy link
Member Author

rvalyi commented Sep 7, 2024

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

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

@OCA-git-bot OCA-git-bot merged commit 145f159 into OCA:14.0 Sep 7, 2024
5 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

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

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.

5 participants